Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create_array creates explicit groups #2795

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

d-v-b
Copy link
Contributor

@d-v-b d-v-b commented Feb 4, 2025

Fixes #2794.

  • init_array used 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_array is modified to create parent groups as needed, and it returns an AsyncArray instance, and accordingly it now takes a config parameter.

A few off-target changes of note, that were necessary to make the above change smoother:

  • The dict form of the ArrayConfig was renamed from ArrayConfigLike to ArrayConfigParams, and the name ArrayConfigLike is now used for the union of ArrayConfig | ArrayConfigParams.
  • Config parsing now happens in AsyncArray.__init__, instead of higher up, since we were already accepting None in that function this is a simplifying change.
  • create_array tests are grouped in a test class. I combined some redundant tests.

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Feb 4, 2025
@github-actions github-actions bot removed the needs release notes Automatically applied to PRs which haven't added release notes label Feb 4, 2025
@d-v-b
Copy link
Contributor Author

d-v-b commented Feb 4, 2025

i can build the docs locally, so I'm confused why the build in ci is failing. Any ideas @dstansby ?

@d-v-b d-v-b marked this pull request as ready for review February 4, 2025 13:52
@dstansby
Copy link
Contributor

dstansby commented Feb 4, 2025

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?

@d-v-b
Copy link
Contributor Author

d-v-b commented Feb 4, 2025

it looks like in the last 20 hours all docs builds for PRs against main have failed with warnings like

[AutoAPI] Reading files... [  2%] /home/docs/checkouts/readthedocs.org/user_builds/zarr/checkouts/2795/src/zarr/_version.py
WARNING: Unable to read file: /home/docs/checkouts/readthedocs.org/user_builds/zarr/checkouts/2795/src/zarr/_version.py [autoapi.not_readable]
[AutoAPI] Reading files... [  3%] /home/docs/checkouts/readthedocs.org/user_builds/zarr/checkouts/2795/src/zarr/creation.py
WARNING: Unable to read file: /home/docs/checkouts/readthedocs.org/user_builds/zarr/checkouts/2795/src/zarr/creation.py [autoapi.not_readable]

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.

@dstansby
Copy link
Contributor

dstansby commented Feb 4, 2025

Are all the dependencies being installed the same between a passing/failing build too?

@d-v-b
Copy link
Contributor Author

d-v-b commented Feb 4, 2025

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.

@d-v-b
Copy link
Contributor Author

d-v-b commented Feb 4, 2025

since #2796 is failing for the same reasons, I feel confident that nothing in this PR is actually causing the docs build problems.

@d-v-b d-v-b requested review from jhamman and dcherian February 4, 2025 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zarr v3 still generates implicit groups
2 participants