[MAINT] bump to nilearn >= 0.12.1 #944
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates the project’s dependencies to require nilearn>=0.12.1 (along with matching transitive dependencies) and refactors private nilearn import paths to align with the changes in the nilearn 0.12.1 API. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Switching to nilearn’s private modules (cache_mixin, niimg_conversions) risks breakage on future releases—consider using public API functions or providing fallback imports.
- Removing the upper bound on nilearn (>=0.12.1) may expose you to breaking changes in future major versions—consider adding a conservative max version or verifying compatibility in CI.
- Bumping nibabel from >=3.x to >=5.x is a significant upgrade; double-check that there are no breaking changes affecting existing I/O workflows.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Switching to nilearn’s private modules (cache_mixin, niimg_conversions) risks breakage on future releases—consider using public API functions or providing fallback imports.
- Removing the upper bound on nilearn (>=0.12.1) may expose you to breaking changes in future major versions—consider adding a conservative max version or verifying compatibility in CI.
- Bumping nibabel from >=3.x to >=5.x is a significant upgrade; double-check that there are no breaking changes affecting existing I/O workflows.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| try: | ||
| # nilearn>0.10.3 | ||
| from nilearn._utils.niimg_conversions import check_same_fov | ||
| except ImportError: | ||
| # nilearn < 0.10.3 | ||
| from nilearn._utils.niimg_conversions import _check_same_fov as check_same_fov | ||
|
|
There was a problem hiding this comment.
not necessary if you don't want the version bump but only fixing issues related to private functions 'namespace' in 0.12.1
There was a problem hiding this comment.
some of those may not be needed if you do not pin to nilearn>=0.12.1
|
test are failing because nilearn 0.12.1 does not support python 3.8 that had gone through EOL almost a year ago |
|
Thanks for the PR! @Remi-Gau could you rebase against main? that should fix the python 3.8 issue. I've been operating under the notion to keep NiMARE permissive to several versions of nilearn, but if you think there are critical fixes in version 0.12.1 that are pertinent to how we use nilearn/y'all have a recommended software cycle of deprecating older versions of nilearn, then I would be convinced to bump up the minimum version of nilearn. Additionally, if we bump up nilearn, there is a minimal dependency section in |
done |
hmm... let me double-check how nilearn is used in here... |
|
maybe the version bump of nilearn is not so necessary, but you would at least need to change the import changes from _utils to avoid things going "boink" with nilearn >= 0.12.1 |
Closes #943
Changes proposed in this pull request:
Summary by Sourcery
Ensure compatibility with nilearn>=0.12.1 by updating dependency version bounds and adapting private nilearn imports
Enhancements:
Build: