-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Slightly Amend Zarr Encoding Specification Doc #8749 #11013
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: main
Are you sure you want to change the base?
Conversation
Updating feature branch with latest commits to upstream main
TomNicholas
left a comment
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.
Great, thank you!
shoyer
left a comment
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.
thanks @eshort0401 !
doc/internals/zarr-encoding-spec.rst
Outdated
| Because of these encoding choices, Xarray cannot read arbitrary Zarr arrays, but only | ||
| Zarr data with valid dimension metadata. Xarray supports: | ||
| Because of these encoding choices, Xarray cannot read arbitrary Zarr groups, but only | ||
| Zarr groups with valid dimension metadata. Xarray supports: |
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 would say "Zarr groups containing arrays with valid dimension metadata" here
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.
doc/internals/zarr-encoding-spec.rst
Outdated
| 1. Zarr V3 groups with ``dimension_names`` metadata | ||
| 2. Zarr V2 groups with ``_ARRAY_DIMENSIONS`` attributes |
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 these should still be "arrays" not "groups" here, because the metadata/attributes are on the array, not the group
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.
Fixed (0b067d9) - thank you!
_ARRAY_DIMENSIONSxarray's special zarr attribute #280 #8749whats-new.rstSlight documentation amendments following discussion in #8749 and #11006.
xarray.backends.zarr._get_zarr_dims_and_attrsproviding the link to the (very helpful) Xarray encoding specification - I missed this document when I first looked at this code!Additional Notes
_ARRAY_DIMENSIONSxarray's special zarr attribute #280 #8749 mentions improving the error message given when dimension name metadata is missing, but this was implemented in Ensure KeyError raised for zarr datasets missing dim names #10025._ARRAY_DIMENSIONSxarray's special zarr attribute #280 #8749.