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.
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
Compatibility for zarr-python 3.x #9552
Compatibility for zarr-python 3.x #9552
Changes from 20 commits
c54a052
483eb7f
40a746c
6c8d2bb
531b521
849df40
88bd64b
ef1549a
20c22bd
6087e5e
15fe55e
8e06bc7
f22100e
f8c427f
594d36d
046d37e
6b0ca62
d315583
389cc82
1fe409a
7c29ea6
efb66dd
90c0ae6
9b3c288
3717391
8d16bb2
bd978b0
34c4c24
118e50e
e6e2066
1d1d9cb
9089508
ea00308
a330e4b
bde42ee
b15705d
38f43b9
1cfc458
af1a0b8
d9d6fee
4c54371
1ce8878
0c2e260
fc2738a
0e47c3f
5b39f42
7d9fc05
0fa94ee
c2fd6f1
ac2ef29
c6be467
5b5b77f
4f07eb7
5151bc2
00c62d7
e0390a5
2e7ec07
26081d4
0350056
a38bff6
08f0594
0e81edf
55d852d
3491137
5bf5f2a
f2f9fff
a84fa79
c280f24
9fec1d6
04c017e
625591e
3795b07
ea2cb57
4f617d2
d1e3c73
45d5a78
f208c39
968217c
45a37f6
c15e856
c10bfc0
82e6a6d
d752693
0fd4103
c2a47a1
26b2661
1d73d36
be79e88
ff0f2c0
5f37042
268e3eb
1abb2ba
7682bf4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can this be removed at some point in the future? If so, it would be good to add a TODO
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'll look more closely later, but for now I think this will be required, following a deliberate change in zarr v3 consolidated metadata.
With v2 metadata, I think that consolidated happened at the store-level, and was all-or-nothing. If you have two Groups with Arrays, the consolidated metadata will be placed at the store root, and will contain everything:
With v3, consolidated metadata is scoped to a Group, so we can provide the group we want to consolidated (the zarr-python API does support "consolidate everything in the store at the root", but I don't think we want that because you'd need to open it at the root when reading, and I think it's kinda where for
ds.to_zarr(group="A")
to be reading / writing stuff outside of theA
prefix).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.
Potentially it would make sense to have two versions of consolidated metadata:
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.
Agreed. zarr-developers/zarr-specs#309 has some discussion on adding a
depth
field to the spec for consolidated metadata. That's currently implicitlydepth=None
, which is everything below a group.depth=0
or1
would be just the immediate children. That's not standardized or implemented anywhere yet, but the current implementation is forwards compatible and it shouldn't be a ton of effort.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.
am I missing something or is this a circular pop replace?