Skip to content

Add StoreV3 support to Array (zarr v3 support part 3) #895

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 22 commits into from

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Nov 30, 2021

This PR builds on top of #894, adding StoreV3 support to the core Array object.

For the tests, the primary difference is that the V3 base test class inherits from the existing TestArrayWithPath instead of TestArray since specifying the path is required for v3. Mostly it is the create_array and hex_digest methods that needed to be overridden.

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 #895 (74369a4) into master (de7858a) will decrease coverage by 1.71%.
The diff coverage is 88.53%.

❗ Current head 74369a4 differs from pull request most recent head 7c3736d. Consider uploading reports for the commit 7c3736d to get more accurate results

@@            Coverage Diff             @@
##           master     #895      +/-   ##
==========================================
- Coverage   99.94%   98.22%   -1.72%     
==========================================
  Files          32       34       +2     
  Lines       11216    13029    +1813     
==========================================
+ Hits        11210    12798    +1588     
- Misses          6      231     +225     
Impacted Files Coverage Δ
zarr/tests/test_core.py 98.73% <29.41%> (-1.27%) ⬇️
zarr/_storage/store.py 73.73% <58.99%> (-26.27%) ⬇️
zarr/tests/test_storage.py 99.92% <66.66%> (-0.08%) ⬇️
zarr/storage.py 93.96% <77.83%> (-6.04%) ⬇️
zarr/meta.py 91.92% <81.74%> (-8.08%) ⬇️
zarr/attrs.py 97.05% <92.68%> (-2.95%) ⬇️
zarr/core.py 99.73% <96.49%> (-0.27%) ⬇️
zarr/tests/test_storage_v3.py 96.86% <96.86%> (ø)
zarr/tests/test_core_v3.py 98.75% <98.75%> (ø)
zarr/tests/test_attrs.py 100.00% <100.00%> (ø)
... and 4 more

zarr_version should not be in the array metadata, only the base store metadata

compressor should be absent when there is no compression
classmethods adapted from zarrita code
@pep8speaks
Copy link

pep8speaks commented Dec 17, 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-17 22:01:32 UTC

@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