-
Notifications
You must be signed in to change notification settings - Fork 65
parse_url_deprecation #476
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: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #476 +/- ##
==========================================
+ Coverage 87.02% 87.13% +0.11%
==========================================
Files 13 13
Lines 1772 1772
==========================================
+ Hits 1542 1544 +2
+ Misses 230 228 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I wonder if it wouldn't be safer to mark this method deprecated to warn users clearly and then create another method strictly for reading. |
|
The reading API is what I'm undecided about. Whereas this is currently: We could do something like: And something similar for labels via names. |
|
With something simple like this: We can do this... |
As discussed at #474 (comment), we have various issues with
parse_url()as it incompletely wrapszarr.open_group().See #376, #445, #471, #498.
This PR updates docs and tests to avoid using
parse_url().Instead of multiple lines for writing, we can now just do:
NB: this is not a breaking change as the
groupargument can be a path(str)or a group.For reading, this is currently less concise than with
parse_url()since we don't create an itterableReader()- see discussion below...