-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix: update numpy dependency constraints and remove platform-specific… #156
base: main
Are you sure you want to change the base?
Conversation
… entries from lock files
WalkthroughThe pull request introduces modifications to multiple configuration files for the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🔇 Additional comments (3)config/ruff.toml (1)
The addition of the "NPY201" rule is well-documented and appropriately categorized under a clear "MIGRATION TO NUMPY2" section. This aligns perfectly with the PR's objective of updating NumPy dependency constraints and will help catch any compatibility issues early. pixi.toml (2)
The update to ruff >=0.4.8 is appropriate and necessary to support the newly added NPY201 rule for NumPy 2.0 migration checks. Line range hint The PR objectives mention removing platform-specific entries from lock files, but the platforms section still contains entries for "linux-64" and "osx-arm64". Could you clarify if these should be removed as part of this PR? Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml
(1 hunks)
pyproject.toml
Outdated
@@ -21,7 +21,7 @@ requires-python = ">=3.10,<4" | |||
dependencies = [ | |||
"h5py>=3.11.0,<4", | |||
"joblib>=1.4.2,<2", | |||
"numpy>=1.26.4,<2", | |||
"numpy>=1.26.4", |
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.
💡 Codebase verification
🛠️ Refactor suggestion
Consider maintaining an upper bound for numpy dependency
Based on the codebase analysis, the project heavily relies on numpy's array operations, particularly for:
- Medical image transformations and resampling
- Array manipulations (transpose, reshape, where)
- Mathematical operations (exp, log, floor)
- Type handling and array creation (zeros, array, asarray)
Given that:
- The codebase extensively uses numpy's core functionality
- numpy 2.0 introduces breaking changes in array creation, dtypes, and type handling
- Medical image processing requires high reliability and reproducibility
- Other dependencies maintain upper bounds (h5py<4, pandas<3)
Recommend maintaining the upper bound:
- "numpy>=1.26.4",
+ "numpy>=1.26.4,<2",
Key reasons:
- Extensive use of numpy array operations in critical image processing functions
- Medical software requires strict version control for reliability
- Consistency with other dependency constraints in the project
- Protection against breaking changes in numpy 2.0
🔗 Analysis chain
Consider maintaining an upper bound for numpy dependency
The removal of the upper version bound for numpy (numpy>=1.26.4
) deviates from the project's consistent dependency management pattern, where other dependencies maintain upper bounds (e.g., h5py>=3.11.0,<4
, pandas>=2.2.2,<3
). This change could introduce risks:
- numpy 2.0 introduces breaking changes that might affect the medical image processing pipeline
- Unbounded dependencies in medical software can compromise stability and reproducibility
- Inconsistent version constraints make dependency resolution less predictable
Consider maintaining the upper bound:
- "numpy>=1.26.4",
+ "numpy>=1.26.4,<2",
Let's verify the potential impact of numpy 2.0:
Let's analyze how numpy is used in the codebase to better understand the potential impact of removing the upper bound constraint.
Let's analyze the actual numpy operations used in the codebase to better understand the potential impact of numpy 2.0 changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for numpy API usage that might be affected by 2.0 changes
rg -l 'numpy' src/ | xargs rg -A 2 'np\.(matrix|bool_|float_|int_|unicode_|string_|deprecate|dtype|typeDict)'
Length of output: 109
Script:
#!/bin/bash
# Let's try a broader search for numpy usage patterns
rg -l 'numpy' src/
# Then search for specific numpy imports and usage
rg 'import numpy|from numpy' src/
# Also check for potential numpy type annotations
rg ':(?: +)?np\.' src/
Length of output: 2234
Script:
#!/bin/bash
# Let's check for specific numpy API usage that might be affected by 2.0 changes
rg 'np\.' src/ -A 2 | grep -v "import"
# Also check for array operations that might be sensitive to numpy changes
rg 'np\.(array|asarray|zeros|ones|empty|full|arange|linspace|reshape|transpose|concatenate|stack|vstack|hstack|split|where|logical_|bitwise_|add|subtract|multiply|divide)' src/
Length of output: 27818
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #156 +/- ##
=======================================
Coverage 44.03% 44.03%
=======================================
Files 48 48
Lines 3540 3540
=======================================
Hits 1559 1559
Misses 1981 1981 ☔ View full report in Codecov by Sentry. |
… entries from lock files
Summary by CodeRabbit
numpy
, allowing for versions 2.0.0 and above, enhancing compatibility.ruff
dependency version to ensure access to the latest improvements.