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
[MAINT] refactor tests regions extractions #3931
Conversation
Remi-Gau
commented
Aug 28, 2023
•
edited
edited
- relates to simplify "complex" tests #3570
- extract tests for warnings
Those new tests were added by #3732 Will try to implement some CI check that makes sure that the length (number of lines) of our tests does not increase beyond a certain threshold. |
👋 @Remi-Gau Thanks for creating a PR! Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft. Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.
For new features:
For bug fixes:
We will review it as quick as possible, feel free to ping us with questions if needed. |
Codecov Report
@@ Coverage Diff @@
## main #3931 +/- ##
=======================================
Coverage 91.77% 91.77%
=======================================
Files 134 134
Lines 15750 15750
Branches 3283 3283
=======================================
Hits 14454 14454
Misses 752 752
Partials 544 544
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM and thanks for adding that flake8 check
actually something is strange: the flake8 check should have failed at some point I think. |
@@ -11,7 +11,7 @@ exclude = | |||
nilearn_cache | |||
venv, | |||
doc/_build | |||
--select = D,E,F,W,C90 | |||
--select = D,E,F,W,C90,CFQ |
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.
check for errors regarding code quality of functions
@@ -41,7 +41,7 @@ per-file-ignores = | |||
examples/*/*: D103, D205, D301, D400 | |||
# - docstrings rules that should not be applied to doc | |||
doc/*: D100, D103, F401 | |||
ignore = D105, D107, E402, W503, W504, W605, BLK100 | |||
ignore = D105, D107, E402, W503, W504, W605, BLK100, CFQ003 |
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.
we got some non pure functions in the test suite: silencing this error for now.
will try to address later
./nilearn/decoding/tests/test_same_api.py:81:1: CFQ003 Function "test_same_energy_calculus_pure_lasso" is not pure.
./nilearn/decoding/tests/test_same_api.py:125:1: CFQ003 Function "test_graph_net_and_tvl1_same_for_pure_l1" is not pure.
./nilearn/decoding/tests/test_same_api.py:163:1: CFQ003 Function "test_graph_net_and_tvl1_same_for_pure_l1_BaseSpaceNet" is not pure.
./nilearn/decoding/tests/test_same_api.py:204:1: CFQ003 Function "test_graph_net_and_tvl1_same_for_pure_l1_logistic" is not pure.
./nilearn/decoding/tests/test_same_api.py:232:1: CFQ003 Function "test_graph_net_and_tvl1_same_for_pure_l1_logistic_spacenet_classifier" is not pure.
./nilearn/decoding/tests/test_same_api.py:266:1: CFQ003 Function "test_graph_net_and_tv_same_for_pure_l1_another_test" is not pure.
./nilearn/decoding/tests/test_space_net.py:441:1: CFQ003 Function "test_space_net_alpha_grid_pure_spatial" is not pure.
run: python -m pip install --upgrade pip flake8 flake8-docstrings flake8-use-fstring flake8-functions | ||
|
||
- name: Run Flake8 on whole file | ||
shell: bash {0} | ||
run: flake8 --verbose . | ||
|
||
- name: Ensure that tests functions are not too long |
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.
this checks code quality of the function of the whole code base (with the very lax constraints we have for now in the flake8 config)
and specifically checks for too long functions in the test suite