-
Notifications
You must be signed in to change notification settings - Fork 4
skpkg: fix migrated tests #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ponents Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
…ectories Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Currently, it seems that while running 'test_nmf_mapping_code' the actual output is different than the expected output causing the tests to fail. It also probably isn't a formatting issue, as after reverting the pre-commit changes to the expected output the tests still fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a few suggestions.
@@ -15,6 +15,7 @@ repos: | |||
hooks: | |||
- id: check-yaml | |||
- id: end-of-file-fixer | |||
exclude: ^tests/output/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove these excludes?
tests/test_NMF_analysis_code.py
Outdated
@@ -12,12 +12,12 @@ | |||
data_dir = os.path.join(dir, "data/synthetic_r_vs_gr") | |||
|
|||
test_map = [ | |||
([data_dir, "--xrange", "5,10"], "output_1", "Number of components: 3\n"), | |||
([data_dir], "output_2", "Number of components: 3\n"), | |||
([data_dir, "--xrange", "5,10"], "output_1", "Number of components: 10"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than change the number of components from 3 to 10 in the output, maybe set the number of components to 3 in CLI inputs? I am not sure the flag for that, but it will be in the arg parser.
Signed-off-by: Dasun Abeykoon <Dasun20202020@gmail.com>
Since the plan is to fix the tests later on after the migration process, I'll just close this PR for now |
No description provided.