Implement support for structured and struct zarr-extension defined dtypes

[BrianMichell]

Updates for consistency with the zarr-extension for structured types.

Implements legacy read-only for structured type and support for new struct definition.

TODO:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/user-guide/*.md
• Changes documented as a new file in changes/
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[d-v-b]

this looks great, thanks for this. Instead of modifying the existing structured data type, can we make struct as a subclass of that data type, and register it? That means people who need the legacy dtype can still use it (by overriding the registry)

[BrianMichell]

this looks great, thanks for this. Instead of modifying the existing structured data type, can we make struct as a subclass of that data type, and register it? That means people who need the legacy dtype can still use it (by overriding the registry)

Should be good to go now!

[BrianMichell]

@d-v-b Is there anything I can do to move this PR forward?

[codecov]

Codecov Report

❌ Patch coverage is 92.53731% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.89%. Comparing base (c966cfa) to head (1a6e371).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/zarr/codecs/bytes.py	60.00%	4 Missing ⚠️
src/zarr/core/dtype/npy/structured.py	98.18%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3781      +/-   ##
==========================================
- Coverage   92.98%   92.89%   -0.09%     
==========================================
  Files          87       87              
  Lines       11246    11309      +63     
==========================================
+ Hits        10457    10506      +49     
- Misses        789      803      +14     

Files with missing lines	Coverage Δ
src/zarr/core/array.py	97.64% <100.00%> (ø)
src/zarr/core/dtype/__init__.py	100.00% <100.00%> (ø)
src/zarr/dtype.py	100.00% <ø> (ø)
src/zarr/core/dtype/npy/structured.py	92.56% <98.18%> (-6.37%)	⬇️
src/zarr/codecs/bytes.py	92.75% <60.00%> (-5.58%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[d-v-b]

@d-v-b Is there anything I can do to move this PR forward?

hi @BrianMichell thanks for the ping, I will get a review in soon. since numpy is handling the actual data type, I think we will ask much less of you than the tensorstore devs :)

[d-v-b]

why do we need to modify the Structured data type at all? I would think we can keep it exactly as-is, but do not register it in the data type registry. Users who need backwards compatibility can explicitly unregister the new data type, and register the old one.

[BrianMichell]

I made this design choice to support transparent r+ of Structured dtype and also r+/w for Struct. This implementation should support my usecase where we have petabyte scale Structured legacy data and expect to move forward with generating new data using Struct without requiring either a remaster or client code to register/unregister on the fly.

[d-v-b]

if we remove Structured from the registry, and Struct accepts data_type metadata that would have worked for Structured, then doesn't this achieve your goal? The idea is that the input type for Struct is wider than the input type for Structured, so any valid Structured would also be a valid Struct data type. But if for some reason someone needs to use the old Structured data type, it is still there, untouched. They just have to opt in to using it.

edit, I mixed up Struct and Structured

[BrianMichell]

I see what you're getting at. Made that update!