Skip to content

remove implicit groups#292

Merged
joshmoore merged 5 commits intozarr-developers:mainfrom
d-v-b:no_implicit_groups
Aug 26, 2024
Merged

remove implicit groups#292
joshmoore merged 5 commits intozarr-developers:mainfrom
d-v-b:no_implicit_groups

Conversation

@d-v-b
Copy link
Contributor

@d-v-b d-v-b commented Apr 12, 2024

this PR removes support for implicit groups (Zarr groups with no metadata documentation) from the spec. See #291 for the motivation. Besides changing the content of the text, I don't know what else to change, so I'm keeping this a draft for now.

A mergeable version of this PR would include a section explaining what implicit groups are, why they were added previously, and why they are now removed. I am happy to add this if it looks like this PR has legs.

Related discussion: #184

@sanketverma1704
Copy link
Member

@d-v-b, is this PR ready for review?

@d-v-b
Copy link
Contributor Author

d-v-b commented May 23, 2024

@MSanKeys963 yes!

@d-v-b d-v-b marked this pull request as ready for review May 23, 2024 13:03
@d-v-b
Copy link
Contributor Author

d-v-b commented Jul 23, 2024

how can we move forward with this? I think it's important to make these changes quickly, before we get skew between implementations.

@jhamman
Copy link
Member

jhamman commented Aug 21, 2024

@zarr-developers/steering-council - this PR has sat without review for 4 months. Can we move it forward please?

jbms
jbms previously approved these changes Aug 21, 2024
rabernat
rabernat previously approved these changes Aug 21, 2024
Copy link
Contributor

@rabernat rabernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the push @jhamman.

@joshmoore
Copy link
Member

@d-v-b: do you want to take a look at the conflict?

@d-v-b d-v-b dismissed stale reviews from rabernat and jbms via 33268dc August 22, 2024 09:33
@d-v-b
Copy link
Contributor Author

d-v-b commented Aug 22, 2024

@d-v-b: do you want to take a look at the conflict?

done!

Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @d-v-b.

Merging as an agreed upon "adjustment" of v3 needed to a "spec bug" introduced by removing the concept of a root directory without re-evaluating the explicit/implicit concept.

This is a change that all implementors need to be aware of. See the list of "Changes after Provisional Acceptance" for other such items.

cc: @zarr-developers/implementation-council

@bjoernthiel
Copy link

@joshmoore Please add the following line directly after "Group metadata":
Each Zarr group in a hierarchy must have a group metadata document, named zarr.json.

@d-v-b
Copy link
Contributor Author

d-v-b commented Jun 3, 2025

@bjoernthiel is this section not sufficient for your purposes?

- The array_ or group_ metadata document for the root of a Zarr hierarchy_ is
stored under the key ``zarr.json``.
- The metadata document of a non-root array or group with hierarchy path ``P``
is obtained by stripping the leading ``/`` of the path and appending
``/zarr.json``. For example, the metadata document of an array or group with
path ``/foo/bar`` is ``foo/bar/zarr.json``.

@d-v-b
Copy link
Contributor Author

d-v-b commented Jun 3, 2025

aha, for symmetry we should have something like the following statement:

Each Zarr array in a hierarchy must have an array metadata document, named
``zarr.json``.

in the group section.

@joshmoore
Copy link
Member

@bjoernthiel: #350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants