ManifestArray should use zarr-python's ArrayV3Metadata

[abarciauskas-bgse]

En route to replacing ZArray custom class with zarr-python's ArrayV3Metadata, we first need to support ManifestArrays using ArrayV3Metadata.

In order to achieve #411 in an incremental way, my proposal is to first support ArrayV3Metadata as well as ZArray in ManifestArray's, so that we can then incrementally refactor the readers and writers to implement or use ManifestArray's using ArrayV3Metadata while still having passing tests. Once all readers and writers using ManifestArrays are refactored, we can remove the custom ZArray class.

As suggested in #411, this will include renaming the ManifestArray zarray property to metadata.

See also #423 for a reference on how codec pipelines may be handled.

Thoughts welcome, cc @mpiannucci @TomNicholas

[TomNicholas]

Splitting out refactoring ManifestArray and refactoring the readers is a good idea.

my proposal is to first support ArrayV3Metadata as well as ZArray in ManifestArray's,

However I don't think you should try to support both at once inside ManifestArray - that's going to be really hard and messy.

Instead, you should change ._zarray: ZArray -> ._metadata: ArrayV3Metadata in one go, but avoid having to refactor the readers just by creating some short-lived function

def zarray_to_v3metadata(zarray: ZArray) -> ArrayV3Metadata:
    ...

and calling that in the readers just before they create a ManifestArray.

while still having passing tests

If you wanted to avoid changing test assertions (that would now have to check .metadata instead of checking .zarray), you could simply do the reverse, using a function v3metadata_to_zarray. Then you have minor changes in the tests, but the actual asserts are the same (for now, to be refactored either now or later).

[abarciauskas-bgse]

That makes sense! Thanks @TomNicholas

[abarciauskas-bgse]

FYI I'm working on this in manifest-arrays-use-arrayv3metadata, not quite ready for a draft PR as I'm still working through the implementation and tests. Hoping to finish tomorrow 🤞🏽

[ayushnag]

Amazing! FYI @TomNicholas this is also required for the Manifest/Functional Store idea from here and here. The Store can easily handle when key=0.1.9 for example (using the ManifestArray). However a Store can also receive a get() request for key=zarr.json which means the Store needs to be able to turn a manifest array into zarr v3 metadata.

[abarciauskas-bgse]

I have opened a draft PR: #429 but as you can see in the description and failing tests there is still a bit of work to do.

[abarciauskas-bgse]

Closing via #429