Skip to content

Conversation

@eshort0401
Copy link

Slight documentation amendments following discussion in #8749 and #11006.

  • I add a few lines to communicate that Xarray checks all three supported conventions; first Zarr v3, then Xarray flavoured Zarr v2, and finally NCZarr, raising an error if no dimension names are found.
  • I make a few additional word choice edits for consistency and clarity, e.g. replace "Zarr arrays" with "Zarr groups" in the "Compatibility and Reading" section.
  • I have also added an additional code comment to xarray.backends.zarr._get_zarr_dims_and_attrs providing the link to the (very helpful) Xarray encoding specification - I missed this document when I first looked at this code!

Additional Notes

Copy link
Member

@TomNicholas TomNicholas left a comment

Choose a reason for hiding this comment

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

Great, thank you!

Copy link
Member

@shoyer shoyer left a comment

Choose a reason for hiding this comment

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

thanks @eshort0401 !

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:
Copy link
Member

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

Copy link
Author

Choose a reason for hiding this comment

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

Fixed (0b067d9) - thanks @shoyer!

Comment on lines 65 to 66
1. Zarr V3 groups with ``dimension_names`` metadata
2. Zarr V2 groups with ``_ARRAY_DIMENSIONS`` attributes
Copy link
Member

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

Copy link
Author

Choose a reason for hiding this comment

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

Fixed (0b067d9) - thank you!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lack of resilience towards missing _ARRAY_DIMENSIONS xarray's special zarr attribute #280

3 participants