Fix open(..., mode='w') to create a group #2629
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d-v-b
reviewed
Jan 3, 2025
| @@ -312,7 +312,7 @@ async def open( | |||
| store_path = await make_store_path(store, mode=mode, path=path, storage_options=storage_options) | |||
|
|
|||
| # TODO: the mode check below seems wrong! | |||
| if "shape" not in kwargs and mode in {"a", "r", "r+"}: | |||
| if "shape" not in kwargs and mode in {"a", "r", "r+", "w"}: | |||
There was a problem hiding this comment.
👍
Based on the list of modes, I think the only thing missing here is "w-", so in the future we could also consider simply checking that mode is not "w-", but I don't think we need to do that here.
d-v-b
approved these changes
Jan 3, 2025
|
The test added here is good, but I assume that we have some pre-existing test that is weaker than it should be. A follow-up PR should find that test and fix it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2490
zarr.opendo not allow creating zarr group (regressions since zarr 3.0.0b1) #2490 -- confirmed this test fails on mainI left the ominous todo note above the check because I'm still not sure it's
all the modes for which that code is valid. But it fixes the immediate issue.
TODO:
Add docstrings and API docs for any new/modified user-facing classes and functionsNew/modified features documented in docs/tutorial.rst