Skip to content

Add StoreV3 support to Attributes (zarr v3 support part 2) #894

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

Closed
wants to merge 10 commits into from

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Nov 30, 2021

This PR builds on #874, updating just the Attributes class to support the v3 spec (only commit c42433d is new at this time)

The test cases here take a different approach than in #874, just adding a fixture to parameterize the tests over the zarr versions. This results in relatively small diffs to the test file.

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)

Define the StoreV3 class and create v3 versions of most existing stores

Add a test_storage_v3.py with test classes inheriting from their v2
counterparts. Only a subset of methods involving differences in v3
behavior were overridden.
@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #894 (2cbbe77) into master (de7858a) will decrease coverage by 1.85%.
The diff coverage is 82.35%.

❗ Current head 2cbbe77 differs from pull request most recent head 27579c3. Consider uploading reports for the commit 27579c3 to get more accurate results

@@            Coverage Diff             @@
##           master     #894      +/-   ##
==========================================
- Coverage   99.94%   98.08%   -1.86%     
==========================================
  Files          32       33       +1     
  Lines       11216    12401    +1185     
==========================================
+ Hits        11210    12164     +954     
- Misses          6      237     +231     
Impacted Files Coverage Δ
zarr/_storage/store.py 69.58% <52.51%> (-30.42%) ⬇️
zarr/tests/test_storage.py 99.92% <66.66%> (-0.08%) ⬇️
zarr/meta.py 87.01% <70.63%> (-12.99%) ⬇️
zarr/storage.py 92.91% <73.97%> (-7.09%) ⬇️
zarr/attrs.py 97.05% <92.68%> (-2.95%) ⬇️
zarr/tests/test_storage_v3.py 96.86% <96.86%> (ø)
zarr/tests/test_attrs.py 100.00% <100.00%> (ø)
zarr/tests/test_sync.py 100.00% <100.00%> (ø)
zarr/tests/util.py 100.00% <100.00%> (ø)
... and 1 more

@pep8speaks
Copy link

pep8speaks commented Dec 15, 2021

Hello @grlee77! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-12-15 14:47:20 UTC

avoid pytest error about missing fixture

fix flake8 error related to zarr_version fixture
@grlee77
Copy link
Contributor Author

grlee77 commented Mar 4, 2022

closing, these commits along with additional fixes and tests are incorporated in #898

@grlee77 grlee77 closed this Mar 4, 2022
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.

2 participants