Uh oh!
There was an error while loading. Please reload this page.
Feature: support rectilinear chunk grid extension - #3534
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## main #3534 +/- ##
==========================================
+ Coverage 60.94% 61.65% +0.71%
==========================================
Files 86 86 Lines 10268 10769 +501 ==========================================
+ Hits 6258 6640 +382 - Misses 4010 4129 +119
🚀 New features to boost your workflow:
|
…ature/rectilinear-chunk-grid
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| @dataclass(frozen=True) | ||
| class RectilinearChunkGrid(ChunkGrid): |
There was a problem hiding this comment.
thoughts on just calling this class Rectilinear, and renaming the RegularChunkGrid to Regular? We could keep around a RegularChunkGrid class for compatibility. But I feel like people know these are chunk grids when they import them
There was a problem hiding this comment.
50/50. I think the more descriptive class is useful when looking at a tracebacks. Plus, this is currently in .core so its not meant to be used directly by users.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| @given(data=st.data()) | ||
| async def test_basic_indexing(data: st.DataObject) -> None: | ||
| zarray = data.draw(simple_arrays()) | ||
| @given(data=st.data(), zarray=st.one_of([simple_arrays(), complex_chunked_arrays()])) |
There was a problem hiding this comment.
Because the search space for the standard arrays strategy is so large, i made a different one complex_chunked_arrays that purely checks different chunk grids
with simple_arrays() we are only spending 10% of our time trying RectilinearChunkGrid so using this approach. We should boost number of examples too.
| 2. **Not compatible with sharding**: You cannot use variable chunking together with | ||
| the sharding feature. Arrays must use either variable chunking or sharding, but not both. |
There was a problem hiding this comment.
I hope this is a temporary limitation! There's a natural extension of rectilinear chunk grids to rectilinear shard grids.
There was a problem hiding this comment.
Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
maxrjones
commented
Feb 17, 2026
I tried this out in virtual tiff (virtual-zarr/virtual-tiff#69) and virtualizarr (zarr-developers/VirtualiZarr#877) and am super excited about this feature. I expect that it'll unlock a lot of downstream development and absolutely support releasing it this week as experimental. I think that the experimental status is already effectively documented. My only additional comment beyond my two reviews earlier today is that I'm not convinced of the value of deprecating Thanks for your work on this @jhamman! |
Add vectorized array_indices_to_chunk_dim to eliminate loops
Resolve conflicts in src/zarr/core/array.py: take main's refactored helper functions (_nchunks_initialized, _resize) which are compatible with RectilinearChunkGrid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix _info() crash for RectilinearChunkGrid arrays by catching NotImplementedError from metadata.chunks (displays "<variable>") - Fix spurious FutureWarning in _nchunks_initialized() by using array.metadata.chunks instead of array.chunks - Add empty-chunk validation in _normalize_rectilinear_chunks to reject chunk specs where the last chunk(s) contain no valid data - Remove FutureWarning deprecation on .chunks for RegularChunkGrid per reviewer feedback; .chunks still raises NotImplementedError for RectilinearChunkGrid Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…arr-python into feature/rectilinear-chunk-grid
| @dataclass(frozen=True) | ||
| class RegularChunkGrid(ChunkGrid): |
There was a problem hiding this comment.
I don't think we actually need a regular chunk grid class, since the rectilinear chunk grid is more general.
There was a problem hiding this comment.
I disagree. Downstream libraries may want to use this info to possibly use a fastpath.
There was a problem hiding this comment.
How would that work? Because a rectilinear chunk grid can be regular. So if a library uses RegularChunkGrid to trigger a fastpath, they also need to inspect the could-be-regular chunks of the RectilinearChunkGrid as well, and then we are back to the RegularChunkGrid having no use.
There was a problem hiding this comment.
and the information about the chunk grid an array is using is already contained in the metadata for that array. clients should feel more comfortable inspecting that document than importing internal utility classes from zarr python.
There was a problem hiding this comment.
they also need to inspect
They don't need to. This stuff can be expensive. My input here comes from many years of trying to write optimized dask code. Having to inspect the damned chunks tuple with potentially 10 million elements in Python is quite slow and annoying. We should strive to preserve such context through strong types.
Here is an example: https://github.com/pydata/xarray/pull/9808/changes . This core problem there was that we normalized zarr's nice tuple[int, ...] to dask-style tuple[tuple[int, ...]] and then later added a "nice performance warning" that slowed everything down because of the need to parse through half a million identical ints. 🤦🏾♂️
There was a problem hiding this comment.
They don't need to. This stuff can be expensive.
Inspecting the chunk_grid attribute of the metadata document will never be expensive. We are not going to put 10 million elements in JSON.
There was a problem hiding this comment.
for context, my suggestion is the removal of the RegularChunkGridclass, not the removal of the information that the chunk grid is regular. Those are different things.
| @dataclass(frozen=True) | ||
| class RectilinearChunkGrid(ChunkGrid): |
There was a problem hiding this comment.
many of the methods on this class take the array shape as a parameter. For an array with a constant shape, this is redundant, so we should probably define the array shape as an attribute of this class.
| }, | ||
| } | ||
| def update_shape(self, new_shape: tuple[int, ...]) -> Self: |
There was a problem hiding this comment.
the name of this method is confusing -- it makes me think the chunk grid has a shape attribute. And I feel like we should allow people to define the chunking of the new part of the array. so maybe resize would be a better name in that case
| dtype: ZDTypeLike | None = None, | ||
| data: np.ndarray[Any, np.dtype[Any]] | None = None, | ||
| chunks: tuple[int, ...] | Literal["auto"] = "auto", | ||
| chunks: ChunksLike = "auto", |
There was a problem hiding this comment.
we declare rectilinear chunks and regular chunks both via the chunks attribute, but accessing the chunks attribute of the array raises an exception in the rectilinear case. This is a deviation from the way the rest of the parameters in this function work -- they are all attributes on the array class, and none of them raise exceptions when you access them. The closest thing is shards being None, but that's consistent with the signature of this function.
There was a problem hiding this comment.
also -- we need a way for someone to say "these chunks are regular, and I want to use the regular chunk grid" vs "these chunks are regular, and I want to use the rectilinear chunk grid". I don't think the signature right now supports this.
| "The `chunks` property is not supported for arrays with variable chunk sizes " | ||
| "(RectilinearChunkGrid). Use `chunk_grid` instead to access chunk information." | ||
| ) | ||
| raise NotImplementedError(msg) |
There was a problem hiding this comment.
I'd like "accessing some attributes raises exceptions" to be a last resort, even if it means breaking public API. What if we copied dask, and made chunks a tuple of tuples of edge lengths, one per axis?
There was a problem hiding this comment.
Everyone using Zarr today expects chunks to be a tuple[int]. I think it's significantly worse to silently break that expectation
There was a problem hiding this comment.
yes, it would be a breaking change, and so of course we can't do it silently, but it might also be a welcome change to any downstream library that uses dask with zarr, so IMO we should consider this as a possibility for how we want the zarr array API to eventually look
| @dataclass(frozen=True, kw_only=True) | ||
| class ArrayV2Metadata(Metadata): | ||
| shape: tuple[int, ...] | ||
| chunks: tuple[int, ...] |
There was a problem hiding this comment.
why are we removing this? that's very confusing, from a "model the v2 metadata document" pov, where chunks is an attribute
maxrjones
commented
Mar 14, 2026
I've been building on the work in this PR and the design discussion here to explore an alternative internal architecture for rectilinear chunk grids. The result is a polished proof of concept branch with a design doc, comprehensive tests, and working downstream POCs for xarray, cubed, VirtualiZarr, and virtual-tiff. Branch:maxrjones#5 The approach reuses several components from this PR (RLE helpers, validation logic, test patterns) and tries to address the design feedback raised here. While the existing PR would deliver the long-requested rectilinear chunks feature, I would appreciate at least consideration of the alternative design because I think it has some clean abstractions that could enable future extensions. For example, it would be quite simple to add support for repeating chunk grids. The key architectural differences:
The design doc covers the rationale for these choices, addresses all the review feedback from this PR, and documents what was intentionally deferred (metadata/array separation, resize chunks parameter, tile encoding). I'd suggest reviewing the design doc first to see if the approach makes sense. If it does, I can split the implementation into 5 incremental PRs detailed at the bottom of the design doc. Thanks, @jhamman, for the foundational work in this PR! I really appreciate everything you've done here and found it essential for making architectural trade-offs concrete. I hope y'all will consider the alternative architecture in my branch/design doc. |
Summary
Adds support for RectilinearChunkGrid extension (Zarr v3), enabling arrays with variable chunk sizes per dimension.
Closes:#1595 | Replaces:#1483 | Related: zarr-extensions#25
Key Features
RectilinearChunkGrid
from_array())[[10, 6]]= 6 chunks of size 10Chunk Grid Access
.chunksProperty BehaviorDesign Decisions
ChunksLikeas TypeAliasResolvedChunkSpecas frozen dataclass.chunksraises for rectilinear.chunk_gridRemoved from Earlier Designs
RegularChunks/RectilinearChunkstuple subclasseschunks.lat)ChunksTypeABC hierarchyDeferred / TODO Items
update_shape()optional chunks parametermetadata/v3.py:483chunk_grids.py:1513-1593Review Focus Areas
High Priority:
chunk_grids.py:RectilinearChunkGridclass,ChunksLiketype, RLE expansion/compression,resolve_chunk_spec()metadata/v3.py:update_shape()for rectilinear resize behaviorindexing.py: Variable chunk indexing with binary searcharray.py:.chunksproperty behavior,.chunk_gridpropertyTests:
test_chunk_grids/test_rectilinear.py: Comprehensive unit teststest_chunk_grids/test_rectilinear_integration.py: End-to-end scenariostesting/strategies.py: Hypothesis strategies for property-based testingBreaking Changes
None. Fully backward compatible.
TODO:
docs/user-guide/*.mdchanges/