Skip to content

Conversation

mkitti
Copy link
Member

@mkitti mkitti commented Jun 4, 2025

This is a forward looking branch that builds on top of #182 to test what a Zarr v3 implementation might look like.

The initial goal is read and write valid Zarr v3 arrays that are similar conceptually to Zarr v2 arrays.

mkitti and others added 29 commits March 11, 2025 22:11
This reduces the test diff
This also reduces the test diff
Change VersionedStore to FormattedStore
TODO: Fix Zarr v3 type strings
@mkitti mkitti force-pushed the mkitti-v3-prototype branch from 7051c32 to d4217fb Compare June 4, 2025 00:11
@coveralls
Copy link

coveralls commented Jun 4, 2025

Pull Request Test Coverage Report for Build 15430475317

Details

  • 89 of 340 (26.18%) changed or added relevant lines in 11 files are covered.
  • 3 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-14.8%) to 70.597%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Storage/http.jl 8 11 72.73%
src/ZArray.jl 8 12 66.67%
src/metadata.jl 15 19 78.95%
src/ZGroup.jl 19 24 79.17%
src/Codecs/Codecs.jl 0 6 0.0%
src/Compressors/v3.jl 0 15 0.0%
src/Codecs/V3/V3.jl 0 36 0.0%
src/Storage/formattedstore.jl 32 84 38.1%
src/metadata3.jl 0 126 0.0%
Files with Coverage Reduction New Missed Lines %
src/Compressors/Compressors.jl 1 88.0%
src/Storage/Storage.jl 1 78.05%
src/ZGroup.jl 1 80.21%
Totals Coverage Status
Change from base Build 15325098477: -14.8%
Covered Lines: 982
Relevant Lines: 1391

💛 - Coveralls


See Zarr version 2
"""
struct FormattedStore{V,SEP,STORE <: AbstractStore} <: AbstractStore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it necessary to have the separator as a type parameter? Could this simply be a struct field?

Copy link
Member Author

@mkitti mkitti Jun 12, 2025

Choose a reason for hiding this comment

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

I earlier had tried this approach.

The main question is how much conditional evaluation do you want to do with each call to citostring versus constant propagation.

The separator parameter is really a "chunk key encoding" parameter. There are currently six cases:

  1. Zarr v2, '.' (default)
  2. Zarr v2, '/'
  3. Zarr v3, '.'
  4. Zarr v3, '/' (default)
  5. Zarr v3, V2 chunk key encoding, '.' (default)
  6. Zarr v3, V2 chunk key encoding, '/'

Choose a reason for hiding this comment

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

bump @meggart

@lazarusA lazarusA mentioned this pull request Oct 13, 2025
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.

5 participants