Skip to content

Zarr v3: support root path - #1085

Merged
joshmoore merged 11 commits into
zarr-developers:mainfrom
grlee77:v3-support-root-path
Sep 8, 2022
Merged

Zarr v3: support root path#1085
joshmoore merged 11 commits into
zarr-developers:mainfrom
grlee77:v3-support-root-path

Conversation

@grlee77

Copy link
Copy Markdown
Contributor

closes#1039

v3 spec states path = '/' for arrays gives an array at /meta/root.array.json
path = '/' for groups gives a group at /meta/root.array.json

In this implementation path=None will also result in a root array. Creation routines default to path=None, so this makes it so that the path argument does not have to be manually specified.

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)

v3 spec states path = '/' for arrays gives /meta/root.array.json
path = '/' for groups gives /meta/root.array.json
In this implementation path = None or path = '' will also result in
a root array. Creation routines default to path=None, so this makes
it so that the path argument does not have to be manually specified.
update test_nbytes_stored to handle both v3 and v2 cases properly
@grlee77grlee77 added the bug Potential issues with the zarr-python library label Jul 19, 2022
@grlee77grlee77 changed the title V3 support root pathZarr v3: support root pathJul 19, 2022
@codecov

codecovBot commented Jul 19, 2022

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.94%. Comparing base (ece1810) to head (e8c23c8).
Report is 344 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #1085 +/- ##
=======================================
Coverage 99.94% 99.94% =======================================
Files 34 34 Lines 13846 13853 +7 =======================================
+ Hits 13839 13846 +7 
Misses 7 7 
FilesCoverage Δ
zarr/_storage/store.py100.00% <100.00%> (ø)
zarr/creation.py100.00% <100.00%> (ø)
zarr/hierarchy.py99.78% <ø> (-0.01%)⬇️
zarr/storage.py100.00% <100.00%> (ø)
zarr/tests/test_convenience.py100.00% <ø> (ø)
zarr/tests/test_core.py100.00% <100.00%> (ø)
zarr/tests/test_creation.py100.00% <100.00%> (ø)
zarr/tests/test_hierarchy.py100.00% <100.00%> (ø)

@joshmoore

Copy link
Copy Markdown
Member

@grlee77, thanks for this! Reading over it, it does seem to address the issues that Stephan raised. Do you have anything else for this PR from your side or just need testing and then rolling into a 2.12.x release??

@grlee77

Copy link
Copy Markdown
ContributorAuthor

I think test coverage is pretty good now. It should be ready for review.

@joshmoore

Copy link
Copy Markdown
Member

Rolling into a 2.13 pre-release for better testing together with xarray.

cc: @shoyer

@joshmoore
joshmoore merged commit 2cfee9c into zarr-developers:mainSep 8, 2022
@joshmoorejoshmoore mentioned this pull request Sep 8, 2022
6 tasks
grlee77 added a commit to grlee77/zarr-python that referenced this pull request Sep 21, 2022
path=None support was previously added in zarr-developersgh-1085. This change should have
been made at that time.
joshmoore pushed a commit that referenced this pull request Sep 22, 2022
path=None support was previously added in gh-1085. This change should have
been made at that time.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugPotential issues with the zarr-python library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Why is path required for opening Zarr v3 groups?

2 participants

@grlee77@joshmoore