Conversation
…rom nilearn. The nilearn package is no longer listed as a dependency.
… noted in setup.py, but no cap on package version is enforced to retain flexibility.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved Nilearn usage by adding local implementations for the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
rabies/confound_correction_pkg/utils.py (1)
766-772: Addstacklevelparameter towarnings.warn.The warning will point to the wrong location in the call stack without specifying
stacklevel. Settingstacklevel=2ensures the warning points to the caller of_smooth_arrayrather than this internal function.📝 Suggested fix
if isinstance(fwhm, (int, float)) and (fwhm == 0.0): - import warnings - warnings.warn("The parameter 'fwhm' for smoothing is specified " - "as {0}. Setting it to None " - "(no smoothing will be performed)" - .format(fwhm)) + import warnings + warnings.warn("The parameter 'fwhm' for smoothing is specified " + "as {0}. Setting it to None " + "(no smoothing will be performed)" + .format(fwhm), stacklevel=2) fwhm = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rabies/confound_correction_pkg/utils.py` around lines 766 - 772, The warning emitted when fwhm == 0.0 inside _smooth_array uses warnings.warn without a stacklevel, so the traceback points to this internal helper; update the warnings.warn call that currently warns about fwhm to include stacklevel=2 (e.g., warnings.warn(..., stacklevel=2)) so the warning points to the caller of _smooth_array rather than the utility function.setup.py (1)
144-145: Outdated Python classifiers.The classifiers still reference Python 3.6, which contradicts
REQUIRES_PYTHON = '>=3.9.0, <=3.13'. Consider updating the classifiers to reflect the actual supported Python versions.📝 Suggested fix
'Programming Language :: Python', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.6', + 'Programming Language :: Python :: 3.9', + 'Programming Language :: Python :: 3.10', + 'Programming Language :: Python :: 3.11', + 'Programming Language :: Python :: 3.12', 'Programming Language :: Python :: Implementation :: CPython', 'Programming Language :: Python :: Implementation :: PyPy'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.py` around lines 144 - 145, Update the Python trove classifiers in setup.py to match REQUIRES_PYTHON (REQUIRES_PYTHON = '>=3.9.0, <=3.13'): remove the obsolete 'Programming Language :: Python :: 3.6' entry and add/update entries for 3.9 through 3.13 (e.g., 'Programming Language :: Python :: 3.9', '... :: 3.10', etc.), leaving implementation classifier 'Programming Language :: Python :: Implementation :: CPython' as appropriate so the classifiers accurately reflect supported versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rabies/confound_correction_pkg/utils.py`:
- Around line 783-785: The code raises NotImplementedError when fwhm == 'fast'
but then contains unreachable dead code calling an undefined helper
_fast_smooth_array; remove the unreachable statement `arr =
_fast_smooth_array(arr)` from the branch handling `if isinstance(fwhm, str) and
(fwhm == 'fast')` (referencing the fwhm check and the undefined symbol
_fast_smooth_array) and scan the module for any other references to
_fast_smooth_array to ensure there are no remaining undefined calls or imports.
---
Nitpick comments:
In `@rabies/confound_correction_pkg/utils.py`:
- Around line 766-772: The warning emitted when fwhm == 0.0 inside _smooth_array
uses warnings.warn without a stacklevel, so the traceback points to this
internal helper; update the warnings.warn call that currently warns about fwhm
to include stacklevel=2 (e.g., warnings.warn(..., stacklevel=2)) so the warning
points to the caller of _smooth_array rather than the utility function.
In `@setup.py`:
- Around line 144-145: Update the Python trove classifiers in setup.py to match
REQUIRES_PYTHON (REQUIRES_PYTHON = '>=3.9.0, <=3.13'): remove the obsolete
'Programming Language :: Python :: 3.6' entry and add/update entries for 3.9
through 3.13 (e.g., 'Programming Language :: Python :: 3.9', '... :: 3.10',
etc.), leaving implementation classifier 'Programming Language :: Python ::
Implementation :: CPython' as appropriate so the classifiers accurately reflect
supported versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ed524fa-936c-4b58-89dc-943d94a8b5d1
📒 Files selected for processing (9)
rabies/analysis_pkg/diagnosis_pkg/analysis_QC.pyrabies/analysis_pkg/diagnosis_pkg/diagnosis_functions.pyrabies/confound_correction_pkg/mod_ICA_AROMA/classification_plots.pyrabies/confound_correction_pkg/utils.pyrabies/preprocess_pkg/preprocess_visual_QC.pyrabies/visualization.pyrabies_environment.dev.ymlrabies_environment.ymlsetup.py
💤 Files with no reviewable changes (3)
- rabies_environment.yml
- rabies/preprocess_pkg/preprocess_visual_QC.py
- rabies_environment.dev.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
New Features
Bug Fixes