-
-
Notifications
You must be signed in to change notification settings - Fork 149
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
Test upgrades for pinned dependencies to improve downstream compatibility #1308
Conversation
Pull Request Test Coverage Report for Build 8235036298Details
💛 - Coveralls |
The comment that justifies the pin specifically references `packaging==21.2`, but the latest version of `packaging` is actually `23.2`, i.e. 2 years after the version that required the pin. Given all the years that have passed, I trust that things have resolved themselves such that pip will handle (skip) any incompatible package versions from two years ago.
I'd like to try and include this PR in the next ivadomed patch release if possible, alongside #1313. :) Would you be able to review, @mathieuboudreau? (Thank you in advance! ❤️) |
@kanishk16 I see you've updated this PR! Does this mean you guys have run into issues with the |
Ah shoot I missed this, sorry about that @joshuacwnewton ! Should I take a crack at it now or wait for the dipy situation to resolve itself shortly (hopefully)? |
Ah, sorry! I wasn't clear about the dipy situation -- the dipy stuff is actually dependent on this PR to be fixed. So, taking a crack at this PR now would be most appreciated! :) |
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! I tested locally IVADOMED, and through github actions ADS using these changes: https://github.com/axondeepseg/axondeepseg/actions/runs/8238093554/job/22528232069
I did encounter an issue with ADS, but it was resolved after we updated our own nibabel dependency. So, we'll just have to updated it on our end the next time we bump the ivadomed version/commit that we use!
…4398) ## Description `dipy==1.8` and above cause conflicts with SCT's venv due to ivadomed. More details here: ivadomed/ivadomed#1308 So, for now, we need to pin `dipy<1.6` to avoid breakage every time a new version of `dipy` is released. But, once ivadomed/ivadomed#1308 is merged and a new version of ivadomed is released, we can switch this pin to `dipy!=1.6.*,!=1.7.*` and allow 1.8/1.9/etc. ## Linked issues Fixes #4397.
This incorporates the merged changes from ivadomed/ivadomed#1308, which allow use to upgrade dipy and numpy.
…d versions of `dipy`/`numpy`/`nibabel` (#4332) ## Description Upstream ivadomed changes: ivadomed/ivadomed#1308 The upstream changes enabled a cascade of version upgrades (compared to `master`): - Explicit dependency upgrades (`requirements.txt`): - `nibabel`: `3.2.2` -> `5.2.1` - `dipy`: `1.5.0` -> `1.8.0` - `numpy`: `1.23.5` -> `1.26.4` - Second-order dependency upgrades (held back by previous packages): - `pandas`: `1.4.4` -> `1.5.3` - `pyparsing`: `2.4.7` -> `3.1.2` Given that we've been held back on these versions for so long, some additional changes are needed to address upstream deprecation warnings and errors, mainly due to `numpy`. ## Linked issues Fixes #4319. Mostly addresses #4408, but doesn't address #4408 (comment). --------- Co-authored-by: Mathieu Guay-Paquet <mathieu.guay-paquet@polymtl.ca>
Description
This PR stems from spinalcordtoolbox/spinalcordtoolbox#4319. In that issue, we discovered that ivadomed pins the
3.x
series ofnibabel
releases, while the current major version of thenibabel
package is5.x
. Disallowing newer versions ofnibabel
caused an incompatibility with the newest version of another package that SCT relies on (dipy
).As a quick fix, SCT has pinned an older version of
dipy
. But, ideally, we would upgrade bothdipy
andnibabel
, which requires changes toivadomed
's pinned dependencies.I'm going to start by testing
nibabel==5.x
, and then move on to other dependencies that currently have some sort of upper bound, just to see the limits of whativadomed
can support.Linked issues
Partially addresses spinalcordtoolbox/spinalcordtoolbox#4319.