create_array creates explicit groups#2795
Conversation
… groups and return an asyncarray instead of metadata
|
i can build the docs locally, so I'm confused why the build in ci is failing. Any ideas @dstansby ? |
Have you looked at the difference between the build here, and the last readthedocs build that passed to see if there's any obvious differences that could be causing the issue? |
|
it looks like in the last 20 hours all docs builds for PRs against the only difference I can see between this and the successful builds is the lack of warnings, and thus the lack of failure. That's not super informative to me unfortunately. |
|
Are all the dependencies being installed the same between a passing/failing build too? |
as far as I can tell there are no differences in the docs build process, but it's possible that I missed something. |
|
since #2796 is failing for the same reasons, I feel confident that nothing in this PR is actually causing the docs build problems. |
dcherian
left a comment
There was a problem hiding this comment.
I added a property test. Please take a look at the last commit.
| @staticmethod | ||
| @pytest.mark.parametrize("zarr_format", [2, 3]) | ||
| @pytest.mark.parametrize("path", [None, "", "/", "/foo", "foo", "foo/bar"]) | ||
| async def test_name(store: Store, zarr_format: ZarrFormat, path: str | None) -> None: |
There was a problem hiding this comment.
this is the only test I reviewed. It looks good.
| if array.metadata.zarr_format == 2: | ||
| assert ( | ||
| sync(array.store.get(f"{parent}/.zgroup", prototype=default_buffer_prototype())) | ||
| is not None | ||
| ) | ||
| elif array.metadata.zarr_format == 3: | ||
| assert ( | ||
| sync(array.store.get(f"{parent}/zarr.json", prototype=default_buffer_prototype())) | ||
| is not None | ||
| ) |
There was a problem hiding this comment.
looks good, thanks for adding this. #2665 will include a function that makes retrieving an array or group from a path in storage easy, when that PR lands we no longer need to call store.get here.
Fixes #2794.
init_arrayused to return a metadata object, and it would only create a single metadata document (the source of the bug reported in zarr v3 still generates implicit groups #2794).init_arrayis modified to create parent groups as needed, and it returns anAsyncArrayinstance, and accordingly it now takes aconfigparameter.A few off-target changes of note, that were necessary to make the above change smoother:
ArrayConfigwas renamed fromArrayConfigLiketoArrayConfigParams, and the nameArrayConfigLikeis now used for the union ofArrayConfig | ArrayConfigParams.AsyncArray.__init__, instead of higher up, since we were already acceptingNonein that function this is a simplifying change.create_arraytests are grouped in a test class. I combined some redundant tests.