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

Feature/store metadata methods #1851

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

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented May 7, 2024

This PR takes adds the option for stores to store metadata in whatever container makes sense for the store.

  • adds Store.set_metadata and Store.get_metadata methods
    • moves json serialization/deserialization to Store ABC.
    • these methods can be overridden by stores that don't want to store metadata as JSON. For now, I've illustrated this with the MemoryStore that now keeps metadata objects as plain dictionaries.

toward #1755

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)

- adds Store.set_metadata and Store.get_metadata methods
- moves json serialization/deserialization to Store ABC
@jhamman jhamman mentioned this pull request May 7, 2024
4 tasks
blosc_configuration_json = zarr_json["codecs"][1]["configuration"]
assert blosc_configuration_json["typesize"] == 1
assert blosc_configuration_json["shuffle"] == "bitshuffle"
assert blosc_configuration_json["shuffle"] == BloscShuffle.bitshuffle
Copy link
Member Author

Choose a reason for hiding this comment

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

I threw this in because it makes the test pass but its really not what we want. We were previously letting json.dumps do a lot of work to serialize Python objects. I think this means that we now need to push more of that logic into the to_dict() methods on the Metadata objects.

cc @d-v-b

@jhamman jhamman added this to the 3.0.0.alpha milestone May 16, 2024
@jhamman jhamman modified the milestones: 3.0.0.alpha, 3.0.0 May 24, 2024
@jhamman jhamman mentioned this pull request Jun 5, 2024
6 tasks
@jhamman jhamman changed the base branch from v3 to main October 14, 2024 20:58
@jhamman jhamman modified the milestones: 3.0.0, After 3.0.0 Oct 17, 2024
@dstansby dstansby removed the V3 label Dec 12, 2024
@dstansby dstansby added the needs release notes Automatically applied to PRs which haven't added release notes label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs release notes Automatically applied to PRs which haven't added release notes
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants