-
Notifications
You must be signed in to change notification settings - Fork 0
build: replace bhklab pyradiomics with main pyradiomics through github install #138
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe PR modifies Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
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: 0
🧹 Nitpick comments (2)
pyproject.toml (2)
14-14: Remove or document the commented-out dependency.Leaving the old dependency commented out is ambiguous—it's unclear whether this is temporary, a fallback reference, or an oversight. Either remove the line completely or add a comment explaining why it's retained (e.g., "# Replaced by git dependency on line 45").
- # "pyradiomics-bhklab>=3.1.4,<4",
45-45: Specify a git revision to ensure reproducible builds.The git dependency lacks a
rev,branch, ortag, so it will always fetch from the default branch (likelymain). This creates non-reproducible builds: different users or CI runs at different times could install different versions.Pixi supports specifying
rev,branch, ortagfor git dependencies, so consider pinning to a specific revision:-pyradiomics = { git = "https://github.com/AIM-Harvard/pyradiomics.git" } +pyradiomics = { git = "https://github.com/AIM-Harvard/pyradiomics.git", rev = "main" }Or replace
rev = "main"with a specific tag (e.g.,tag = "v3.1.0") or commit hash to pin the exact version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-12-13T15:12:37.077Z
Learnt from: jjjermiah
Repo: bhklab/readii PR: 83
File: src/readii/io/loaders/images.py:4-6
Timestamp: 2024-12-13T15:12:37.077Z
Learning: The project requires Python 3.10+.
Applied to files:
pyproject.toml
📚 Learning: 2024-12-11T21:53:05.073Z
Learnt from: jjjermiah
Repo: bhklab/readii PR: 74
File: src/readii/negative_controls_refactor/abstract_classes.py:113-119
Timestamp: 2024-12-11T21:53:05.073Z
Learning: The 'readii' project only supports Python 3.10 and above.
Applied to files:
pyproject.toml
📚 Learning: 2024-12-13T16:20:45.016Z
Learnt from: jjjermiah
Repo: bhklab/readii PR: 84
File: src/readii/io/utils/pattern_resolver.py:5-6
Timestamp: 2024-12-13T16:20:45.016Z
Learning: Dependencies on `med-imagetools` in `readii` are acceptable, as the package is intended to work off of `med-imagetools`.
Applied to files:
pyproject.toml
🔇 Additional comments (1)
pyproject.toml (1)
43-45: Verify thatpyradiomicswill be installed for standard pip-based setups.The new dependency is only declared in
[tool.pixi.pypi-dependencies], not in[project.dependencies. This means standardpip install readiiinstallations may not resolvepyradiomics. If Pixi is the primary (or only) package manager for this project, this is fine. Otherwise, consider whetherpyradiomicsshould also be added to[project.dependencies](or whether the git URL should be specified using a PEP 508 direct reference in the standard dependencies section).Verification checklist:
- Is this project distributed and installed exclusively via Pixi, or do users also use pip?
- Should
pyradiomicsbe added to[project.dependencies]as a fallback for pip-based installs?- Can you confirm that the main pyradiomics repository (AIM-Harvard fork) maintains API compatibility with the previous bhklab fork?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
=======================================
Coverage 75.74% 75.74%
=======================================
Files 41 41
Lines 2020 2020
=======================================
Hits 1530 1530
Misses 490 490 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit