Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork366
Feature: support rectilinear chunk grid extension#3534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedOct 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## main #3534 +/- ##==========================================+ Coverage 61.86% 62.18% +0.32%========================================== Files 85 85 Lines 10111 10604 +493 ==========================================+ Hits 6255 6594 +339- Misses 3856 4010 +154
🚀 New features to boost your workflow:
|
…ature/rectilinear-chunk-grid
| when you need to align chunks with existing data partitions. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| when you need to align chunks with existing data partitions. | |
| when you need to align chunks with existing data partitions. | |
| The specification for this chunking scheme can be found[here](https://github.com/zarr-developers/zarr-extensions/tree/main/chunk-grids/rectilinear/). |
This link doesn't resolve yet but it will when the spec is merged.
| With variable chunking, the standard`.chunks` property is not available since chunks | ||
| have different sizes. Instead, access chunk information through the chunk grid: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think it would be better if.chunks just had a different type (tuple of tuples of ints)
| @dataclass(frozen=True) | ||
| classRectilinearChunkGrid(ChunkGrid): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
thoughts on just calling this classRectilinear, and renaming theRegularChunkGrid toRegular? We could keep around aRegularChunkGrid class for compatibility. But I feel like people know these are chunk grids when they import them
Uh oh!
There was an error while loading.Please reload this page.
| ) | ||
| @cached_property | ||
| def_cumulative_sizes(self)->tuple[tuple[int, ...], ...]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nice, this is cached
Uh oh!
There was an error while loading.Please reload this page.
src/zarr/testing/strategies.py Outdated
| chunk_shapes_rle= [ | ||
| [[c,r]forc,rinzip(draw(dim_chunks),draw(repeats),strict=True)] | ||
| for_inrange(ndim) | ||
| ] | ||
| returnRectilinearChunkGrid(chunk_shapes=chunk_shapes_rle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This needs fixing
| @given(data=st.data()) | ||
| asyncdeftest_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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Because the search space for the standardarrays strategy is so large, i made a different onecomplex_chunked_arrays that purely checks different chunk grids
withsimple_arrays() we are only spending 10% of our time tryingRectilinearChunkGrid 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Uh oh!
There was an error while loading.Please reload this page.
(Marking this as a draft for now)
closes:#1595
replaces:#1483
xref:zarr-developers/zarr-extensions#25
TODO:
docs/user-guide/*.mdchanges/