-
Notifications
You must be signed in to change notification settings - Fork 157
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): upper bound zarr at runtime (and in docs + benchmark) #1819
Conversation
Right, the intersphinx mapping is not automatic |
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.
maybe we should add a zarr
extra that people can rely on instead of having to maintain their own pin.
then they can just specify anndata[zarr]
and get the newest supported version
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.
This struck me as a minor version change. I think this solution also would be necessary anyway to give a helpful message.
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.
I think the raise RuntimeError
you added is the most helpful thing for users here. Although maybe it should be an ImportError, it’s not like users can make a change to the runtime parameters to fix it.
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.
The thought crossed my mind, I can update
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.
Also another reason not to add a dep now is that I have this slated for 0.12 anyway with the xarray
object.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1819 +/- ##
==========================================
- Coverage 87.10% 84.63% -2.48%
==========================================
Files 40 40
Lines 6135 6138 +3
==========================================
- Hits 5344 5195 -149
- Misses 791 943 +152
|
…docs + benchmark)
…benchmark) (#1822) Co-authored-by: Ilan Gold <[email protected]>
I would make a release just for this.