Conversation
|
Super excited for this! Just ran into a use-case today that it'll be great for. |
nice! I'll try to get back to this tomorrow. I got a bit behind on development due to travel. Are you able to share anything about the use-case? I'm supposed to demo this feature at our project wrap-up next week and if it's related to NASA data it could be a good collaboration opportunity. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #954 +/- ##
===========================================
- Coverage 89.67% 73.14% -16.53%
===========================================
Files 41 41
Lines 2682 2726 +44
===========================================
- Hits 2405 1994 -411
- Misses 277 732 +455
🚀 New features to boost your workflow:
|
manifests/array.py and friends import zarr.experimental.ChunkGrid unconditionally to support both regular and rectilinear chunk grids, but that module was only added in zarr 3.2.0 (3.1.6, the previous floor, predates it). Bump the dependency floor and the minimum-versions pixi pin to 3.2.1, the first patch release after it.
Adds an array_v3_metadata_rectilinear fixture and tests covering ManifestArray.chunk_grid, and concat/stack/broadcast_to/indexing/ with_fill_value_only behavior on rectilinear-chunked arrays. Confirms concatenating or stacking purely-regular arrays is not unnecessarily promoted to a rectilinear chunk grid. But concatenating or stacking arrays that already have a rectilinear grid currently crashes: check_combinable_zarr_arrays calls manifest_chunk_shape unconditionally, which assumes a regular chunk grid, so the rectilinear-aware branches later in concatenate()/stack() are dead code. The same manifest_chunk_shape assumption also breaks indexing and with_fill_value_only on rectilinear arrays; those are locked in as documented-crash tests for now.
Fixes the manifest_chunk_shape crash in check_combinable_zarr_arrays that made rectilinear-aware concat/stack unreachable: it assumed a regular grid unconditionally. Introduces chunk_grid_sizes (per-axis size, tolerating a rectilinear grid) and full_chunk_edges (always fully expanded, for equality comparisons across grids zarr may or may not classify as regular) in manifests/utils.py, replacing manifest_chunk_shape in the combinability and partial-chunk checks. concatenate() now decides whether the result must become rectilinear by checking every input's regularity and declared chunk size along the concat axis, not just the first array's - so two regular arrays whose chunk sizes genuinely differ along that axis are also handled, not just already-rectilinear inputs. Whenever a rectilinear result would be needed, both concatenate() and stack() check zarr's `array.rectilinear_chunks` config flag first and raise a clear error naming both ways to enable it if it's off, rather than silently producing metadata most zarr tooling can't read. Also fixes stack() inserting a single (1,) edge for a new rectilinear axis regardless of how many arrays were stacked, instead of one size-1 edge per array. Extends the rectilinear test coverage added in the previous commit with cases for: concat/stack succeeding once genuinely mismatched regular chunk sizes are merged, and raising with a clear message when the flag is disabled.
write_virtual_variable_to_icechunk's require_array() call passed metadata.chunks, which raises NotImplementedError for a rectilinear chunk grid. Falls back to chunk_grid_sizes(metadata) only when .chunks itself doesn't apply, since .chunks has sharding-aware behavior (returns the sharding codec's inner chunk shape rather than the outer/shard shape) that chunk_grid_sizes doesn't replicate and create_array needs preserved for sharded arrays. Verified zarr's require_array already accepts a rectilinear chunks= spec (a homogeneous nested list, one edge-tuple per axis) on the currently pinned zarr 3.2.1 - no upstream zarr-python or xarray change needed for this. Also makes appending to an existing array along append_dim raise NotImplementedError with a clear message when either the incoming or existing array has a rectilinear chunk grid, rather than crashing inside check_compatible_arrays with zarr's generic "chunks attribute is only defined for regular chunk grids" error - merging a rectilinear axis's edges across an append boundary isn't implemented yet. Adds synthetic_vds_rectilinear_grid fixture and tests covering a successful basic write/read-back roundtrip and the append_dim guard.
Adds TestConcatRectilinear to test_xarray.py, exercising rectilinear chunk grid promotion through xarray's own dispatch (xr.concat, open_virtual_mfdataset) rather than calling into virtualizarr.manifests.array_api directly as test_array.py does: - concatenating regular ManifestArrays with genuinely different declared chunk sizes along the concat dimension - concatenating already-rectilinear ManifestArrays - the clear error raised when array.rectilinear_chunks is disabled - a full real-file round trip: two HDF5 files written with deliberately different chunksizes encoding, opened and combined via open_virtual_mfdataset, verifying the merged chunk grid is correct
Adds a "Rectilinear chunk grids" section to the data structures explanation doc covering the opt-in flag, ManifestArray.chunk_grid, and current limitations (no append or region writes to Icechunk yet). Updates the FAQ's "homogeneous chunk shapes" restriction and the usage guide's combining-datasets callout to reflect that this restriction is now partially relaxed for concatenation, and adds a release note entry for the unreleased version.
Tightens wording across the FAQ, usage guide, and data structures explanation, and notes that reading rectilinear-chunked stores back with Xarray requires Xarray v2026.09.0.
for more information, see https://pre-commit.ci
Writes a rectilinear-chunked virtual dataset to an in-memory Icechunk store and reads it back with xr.open_zarr, checking both that xarray's own zarr backend can open a rectilinear array and that the loaded values are correct - not just that zarr-python can, which the existing write test already covers. Currently xfails: xarray's open_store_variable calls zarr_array.chunks unconditionally, which raises NotImplementedError for a rectilinear grid. This is blocked on pydata/xarray#11592 (read-only rectilinear support), not yet merged, so nothing on the VirtualiZarr side to fix here - this documents the gap and will start passing once that PR lands and xarray is bumped.
Confirmed locally against xarray PR #11592's branch (TomNicholas/xarray@zarr-rectilinear-chunks-read) that this test passes end-to-end once that PR's read support is available. Leaving it unmarked so it fails loudly with the currently pinned xarray, rather than xfailing, until that dependency is bumped.
|
I've taken over this PR - thanks for starting it @maxrjones ! It now works locally - CI is failing because it requires Xarray support for reading back the rectilinear chunks. That means we should really release Xarray with pydata/xarray#11592 before we release VirtualiZarr with this feature. One non-trivial design decision I've made here is when to actually use rectilinear chunk grids. Generally we probably don't want to just use rectilinear chunk grids by default, since they are often not needed, the metadata scaling is worse, and fewer tools understand the result. But we also don't want concatenation of two different-length chunks to silently change the chunk grid type. So what I've chosen to do for now is only ever use rectilinear chunk grids if the zarr experimental config flag is switched on, and otherwise error as before. Note there are some other VZ to-do's in #12, but I will stack those on top of this PR. |
check_combinable_zarr_arrays's chunk-shape comparison ran before the array-shape check, so two arrays with the same declared chunk size but different overall shapes on a non-concat axis (which changes that axis's boundary-truncated chunk edges) surfaced a "needs a rectilinear chunk grid" error instead of the correct "differing shapes" one. Reorders the checks so shape mismatches are always caught first.
Appending now promotes the destination array's chunk grid to rectilinear when either the incoming or existing array is already rectilinear, or when both are regular but declare genuinely different chunk sizes along the append axis - mirroring concatenate(), gated behind the same array.rectilinear_chunks opt-in check. - num_chunks() counts rectilinear chunks (len(edges)) instead of assuming shape // chunk_size. - check_compatible_arrays() no longer crashes comparing a rectilinear array's declared chunk sizes, and excludes the append axis from the comparison (chunk sizes are allowed to differ there). - resize_array() only calls zarr's Array.resize() when both sides stay regular with the same chunk size. Otherwise it writes the correctly merged chunk edges directly via copy_and_replace_metadata + save_metadata, since zarr's own rectilinear resize logic only ever appends a single new edge covering the whole size increase - wrong whenever the appended array itself has more than one chunk along that axis. Also fixes a related bug the append test suite caught: comparing chunk shapes via full_chunk_edges (shape-dependent boundary-truncated edges) is wrong wherever the two arrays being compared aren't meant to share the same shape, as in a region write comparing a small tile against the full destination array. check_combinable_zarr_arrays and check_compatible_arrays now compare the declared, shape-independent chunk_grid_sizes instead; check_same_chunk_shapes normalizes a bare int against a matching 1-tuple so the comparison still treats a grid zarr auto-classified as regular the same as an equivalent rectilinear declaration. Region writes now raise a clear NotImplementedError for a rectilinear array up front, rather than crashing on chunk_size-based alignment math that has no equivalent for irregular chunk boundaries. Replaces the old blanket "appending is not yet supported for rectilinear arrays" test with tests covering the two promotion cases and the disabled-flag error, and adds a region-write guard test.
Removes stale "appending is not yet supported" wording from the data structures explanation, the usage guide's append section, and the unreleased release notes entry, now that appending can promote an array to rectilinear. Region writes remain unsupported.
# Conflicts: # virtualizarr/manifests/utils.py # virtualizarr/tests/test_manifests/test_array.py
Avoids depending on zarr.core.chunk_grids._is_rectilinear_chunks, a private internal that could change without notice, for the trivial nested-sequence check we actually need.
This is a companion PR to zarr-developers/zarr-python#3802 that adds support for rectilinear chunk grids.
Acceptance criteria:
docs/releases.md*.mdfile underdocs/api