Repository navigation
zarr: read the regular_bounded chunk grid - #1133
rhoadesScholar wants to merge 6 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Zarr v3 "offset" chunk key encoding extension. It refactors chunk key generation into a centralized encodeChunkKey function, updates metadata parsing to validate and extract offset configurations, and adds comprehensive unit and snapshot tests. The feedback suggests a minor improvement in encodeChunkKey to use optional chaining and nullish coalescing for safer and more concise offset resolution.
dbee33e to
397b44b
Compare
|
Ready for review. The encoding is proposed in zarr-developers/zarr-extensions#80; this is the reader side. cc @jbms |
|
@rhoadesScholar we should convert this to a draft until the change is adopted by Zarr |
|
Agreed, converted. The design is being worked out in zarr-developers/zarr-extensions#79; this branch will be reworked to match. |
8b17d21 to
1038d1b
Compare
|
The metadata form is being settled in ZEP 12 (zarr-developers/zeps#72, spec text zarr-developers/zarr-specs#400). This PR stays on the |
The `offset` encoding (zarr-extensions chunk-key-encodings/offset) stores a chunk under the `default` key of its grid coordinate plus a fixed per-dimension integer offset, so a writer can grow an array in the negative direction by whole chunks without renaming chunks. Parse the name and its `offset` (one integer per dimension, `/` separator by default), and add the offset where grid coordinates become keys. The key string composition moves out of the chunk source into `encodeChunkKey` so it can be unit-tested; `default` and `v2` keys are unchanged. Adds an example store that grew by one chunk row in front (keys c/-1/*) to the metadata snapshot tests.
Rework following zarr-extensions#79: the `offset` chunk key encoding is removed. `regular_bounded` is a regular chunk grid with an `origin` (the array coordinate where chunk 0 starts) and a `lower_bound` (the coordinate of the first element), so chunk grid indices may be negative and the first chunk may be partly filled. Chunk coordinates are taken relative to the origin, as the precomputed source does with its voxel offset: the voxel bounds are `[lower_bound - origin, lower_bound - origin + shape)`, the origin is added in the chunk-to-multiscale transform, and the model bounds start at `lower_bound`. Grid positions, and so keys, come out negative where they should without any change to key formatting. Two example stores written by zarr-python are added to the metadata snapshot tests: one grown in front by `Array.prepend`, and one with a non-zero origin on both axes. tensorstore reads both identically.
A regular_bounded array inside an OME multiscale group was placed with `translation` at coordinate 0, which displaced it by lower_bound * scale. The chunk space now maps to an index space that starts at the first element, as OME-NGFF defines it; a bare array keeps its own coordinates through the scale transform. Adds the regular_bounded_0.5 fixture, simple_0.5 grown in front at both levels, whose per-level transforms match the ungrown group.
Moves a mid-file import to the top (E402) and applies ruff format to the two zarr fixture generators.
1038d1b to
0250eca
Compare
Reads arrays on the
regular_boundedchunk grid (zarr-developers/zarr-extensions#81, planned in zarr-developers/zarr-extensions#79). It is a regular grid with anorigin(the array coordinate where chunk 0 starts) and alower_bound(the coordinate of the first element), under which chunk keys may be negative and the first chunk may be partly filled. Writers use it to grow an array in front without moving chunks (zarr-developers/zarr-python#4516, google/tensorstore#296).Chunk coordinates are taken relative to the origin, as the precomputed source does with its voxel offset:
[lower_bound - origin, lower_bound - origin + shape);origin - lower_bound;lower_bound, so its model bounds start there;translationlocates its first element, as the NGFF coordinate convention defines the index space, so the lower bound does not enter the OME transforms.Grid positions, and so keys, come out negative without any change to key formatting. Chunks that extend below
lowerVoxelBoundwere already supported.Before and after. Before,
chunk_gridmust beregular, so such an array fails to parse. After, it opens with the right bounds.Tests.
regular_bounded.spec.ts.prepend. Another has a non-zero origin on both axes, and its snapshot shows the origin in the transform and the permuted voxel bounds. The third is thesimple_0.5OME group grown in front at both levels, with each level'stranslationmoved to its new first element; its per-level transforms come out identical to the ungrown group's.tensorstore reads the two bare stores identically. vitest, the browser snapshot tests (headless Chromium), tsc, eslint, oxlint and prettier all pass. Not verified: rendering voxel data in a browser session.