-
Notifications
You must be signed in to change notification settings - Fork 7
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
Updated numpy to 2.0.2 and other packages to the latest #364
Conversation
'scipy>=1.2.1', | ||
'zarr>=2.16.0', | ||
'numcodecs>=0.12.0', | ||
'scipy>=1.9.3', | ||
'negspy>=0.2.24', | ||
'pandas>=1.1.2', |
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.
Does pandas
also need to be updated? It looks like there's a new major version out as of last year: https://pandas.pydata.org/docs/whatsnew/index.html
CC @keller-mark
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.
Actually, after taking a closer look at the changelog, it does seem like a version bump is necessary to maintain interoperability between pandas/numpy: https://pandas.pydata.org/docs/whatsnew/v2.2.2.html#pandas-2-2-2-is-now-compatible-with-numpy-2-0
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.
@NickAkhmetov thanks for reviewing. I can change it, but there are tests for Python3.8 and Python3.12, so pandas has to be compatible with Python3.8.
Out of curiosity, wouldn't the latest package be downloaded based on the python version?
I have not changed the others' either, as the restrictions seemed fine. can bump those as well (where possible)
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.
It would be able to resolve to the latest compatible pandas version with the existing approach in a greenfield project; however, if we mark Vitessce as being compatible with older versions of pandas
, it could cause downstream issues for users.
For example, if a user's project has a dependency on pandas<2
and they installed vitessce without the change to pandas>=2.0.2
, then the latest version of pandas would not be installed and unexpected runtime errors could arise from whatever incompatibilities pandas<2.0.2
has with numpy>2
. By setting a stricter range, we can preempt those errors by having the package manager tell the user "hey, this other package also needs to be updated for things to work as expected."
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.
Thanks for the explanation. makes sense
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.
Actually - on further reflection, I was overthinking it. If the numpy version is >1.21.1
, we can actually use the previous pandas version number, since the package manager should be able to resolve the appropriate pairing of numpy/pandas versions that should be used in a given circumstance. Apologies for the confusion 😅
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.
Looks good to me! Some non-blocking questions:
- Should this be a minor version bump instead of a patch, since this includes some major dependency version updates?
- This is more of a matter of personal curiosity - is there something specific about the ome-zarr version we're pinned to? There appears to be a new release as of a few months ago, maybe we can set the
ome-zarr
to>=0.8.3
?
@NickAkhmetov thanks for reviewing. for ome-zarr, I did get errors when I bumped the version, so that could be a reason? |
9f3e334
to
c82969c
Compare
c82969c
to
1192135
Compare
@NickAkhmetov I bumped the minor version but tests were failing, reverting it back is still failing the tests (docs build) though the code is the same as the previous successful run. |
8420c39
to
d97a575
Compare
d97a575
to
35798d0
Compare
@NickAkhmetov numpy/numpy#26710 |
Fixes Issue 362