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] nilearn.interface: make private functions used outside of their module public #4168
Conversation
Several functions interfaces/fmriprep seem to be unused in the code base. @htwangtw is this to be expected?
|
👋 @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 ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4168 +/- ##
==========================================
+ Coverage 91.97% 92.00% +0.02%
==========================================
Files 146 145 -1
Lines 16382 16378 -4
Branches 3431 3431
==========================================
+ Hits 15068 15069 +1
+ Misses 771 768 -3
+ Partials 543 541 -2 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Yes - here's how these |
Ah yes. Thanks. |
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, but there are some codecov complaints...
looks like some of those are "false positive". will double check but I think that in any case there are couple of lines in the incriminated files that were not hit by the tests so I will to cover them with some new tests |
OK should improve the coverage of those lines: https://app.codecov.io/gh/nilearn/nilearn/blob/master/nilearn%2Finterfaces%2Fbids%2Fglm.py#L98 Had to do some refactoring of the test suite so probably increase the number of line changes |
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, thx.
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!
Changes proposed in this pull request:
nilearn.interface.bids._utils
and move functions closer to where they are used or into a newnilearn.interface.bids.utils
nilearn.interface.fmriprep
used outside of their module