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

[v3] h5py compat methods on Group #2128

Merged
merged 11 commits into from
Sep 4, 2024

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Aug 27, 2024

This PR adds the h5py compat methods to the Zarr Group API from v2.

The plan described in #1583 was to remove these but after looking into it, a bit more, I came to the conclusion that require_group and require_dataset are actually pretty useful. I'm open to adding deprecation warnings to create_dataset and require_dataset (perhaps replacing the latter with require_array) if we want to phase out the h5py specific language.

closes #1597

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/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@jhamman jhamman marked this pull request as ready for review August 28, 2024 00:04
@jhamman jhamman requested a review from d-v-b August 30, 2024 18:48
@joshmoore
Copy link
Member

In general, big 👍 for the compatibility. cc: @clbarnes of https://github.com/clbarnes/h5py_like fame in case there are any lessons learned, etc. that we should take into account.

Copy link
Contributor

@d-v-b d-v-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to adding deprecation warnings to create_dataset and require_dataset (perhaps replacing the latter with require_array) if we want to phase out the h5py specific language.

+1 to this, ultimately we shouldn't use the term "dataset". But for short-term hdf5 compatibility this PR looks good.

@d-v-b d-v-b mentioned this pull request Sep 3, 2024
6 tasks
@jhamman
Copy link
Member Author

jhamman commented Sep 4, 2024

In general, big 👍 for the compatibility. cc: @clbarnes of https://github.com/clbarnes/h5py_like fame in case there are any lessons learned, etc. that we should take into account.

I'm going to go ahead and merge this but happy to take feedback from @clbarnes here or in #1597.

@clbarnes
Copy link
Contributor

clbarnes commented Sep 4, 2024

Nothing specific to say, just happy if h5py_like was of any use! Definitely agree on throwing PendingDeprecationWarnings whenever these short-term compatibility measures are used, otherwise you'll never be rid of them.

@jhamman jhamman merged commit 3b793c1 into zarr-developers:v3 Sep 4, 2024
26 checks passed
@jhamman jhamman deleted the feature/h5py-compat-group branch September 4, 2024 23:13
@jhamman jhamman added the V3 label Sep 5, 2024
@jhamman jhamman added this to the 3.0.0 milestone Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

h5py compatibility in v3
4 participants