Parametrize Array with v2/v3 metadata

[dstansby]

It has been a longstanding bugbear of mine that there's no easy way to specify a v2 or v3 array type. This especially came up in the context of #3257, which deals specifically with v2/v3 arrays.

This PR ads type parametrization to the Array class.

After this PR, there is lots of improvements to adding overloads to functions and methods that could be made, but to keep review easier I'd like to leave that for a follow up PR.

[codecov]

Codecov Report

❌ Patch coverage is 29.48718% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.93%. Comparing base (e3ee591) to head (e0de52d).

Files with missing lines	Patch %	Lines
src/zarr/core/group.py	4.54%	21 Missing ⚠️
src/zarr/api/asynchronous.py	0.00%	12 Missing ⚠️
src/zarr/api/synchronous.py	0.00%	10 Missing ⚠️
src/zarr/core/indexing.py	0.00%	6 Missing ⚠️
src/zarr/core/array.py	25.00%	3 Missing ⚠️
src/zarr/core/attributes.py	0.00%	1 Missing ⚠️
src/zarr/core/metadata/__init__.py	0.00%	1 Missing ⚠️
src/zarr/core/sync_group.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3304      +/-   ##
==========================================
+ Coverage   61.86%   61.93%   +0.06%     
==========================================
  Files          85       86       +1     
  Lines       10112    10129      +17     
==========================================
+ Hits         6256     6273      +17     
  Misses       3856     3856              

Files with missing lines	Coverage Δ
src/zarr/metadata/migrate_v3.py	98.36% <100.00%> (+0.01%)	⬆️
src/zarr/testing/strategies.py	97.82% <100.00%> (+0.01%)	⬆️
src/zarr/types.py	100.00% <100.00%> (ø)
src/zarr/core/attributes.py	46.15% <0.00%> (ø)
src/zarr/core/metadata/__init__.py	0.00% <0.00%> (ø)
src/zarr/core/sync_group.py	42.85% <0.00%> (ø)
src/zarr/core/array.py	68.59% <25.00%> (-0.04%)	⬇️
src/zarr/core/indexing.py	69.46% <0.00%> (ø)
src/zarr/api/synchronous.py	36.61% <0.00%> (ø)
src/zarr/api/asynchronous.py	72.20% <0.00%> (ø)
... and 1 more

🚀 New features to boost your workflow:

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

[d-v-b]

these imports will prevent zarr.core.array from using anything defined in zarr.types, is that intentional?

[d-v-b]

(outside of a type-checking context, I mean)

[d-v-b]

thinking more about this, I think as long as everything in zarr.types is imported exclusively in if TYPE_CHECKING blocks, then there's no risk of circular imports. we would only have circular imports if we tried to work with types as values outside of type-checking, for example using get_args(ZarrFormat). We can always avoid this by defining a typed constant value somewhere else in the codebase.

[dstansby]

I certinaly didn't think about this! So neither intentional or unintentional. I'm not sure I see a better solution, but open to suggestions.

[d-v-b]

What was the problem that necessitated defining type aliases in the top-level types file? I wonder if there's another way to solve that. This is my only concern about this PR btw, we can get it in soon after this is resolved

[dstansby]

I think there wasn't a problem, and this was a choice 😄 - the circular import mess seems a good reason to move these out and next to the array/group classes. Might be a little while until I find time, so someone else is very free to finish this PR off if I don't get there first.

[d-v-b]

I think these changes are good, as they make it easier to track the v2-ness of v3-ness of the Array class.

But since this PR changes the Array class, and it will also require anyone who annotated variables with the Array type to update that annotation, I think it's important that we get a lot of eyes on this PR.

cc @zarr-developers/core-devs

[d-v-b]

I think these changes are good, as they make it easier to track the v2-ness of v3-ness of the Array class.

But since this PR changes the Array class, and it will also require anyone who annotated variables with the Array type to update that annotation, I think it's important that we get a lot of eyes on this PR.

cc @zarr-developers/core-devs

ping @zarr-developers/core-devs