Skip to content

refactor: remove random subchunk ordering#4011

Open
d-v-b wants to merge 11 commits into
zarr-developers:mainfrom
d-v-b:no-random-subchunk-order
Open

refactor: remove random subchunk ordering#4011
d-v-b wants to merge 11 commits into
zarr-developers:mainfrom
d-v-b:no-random-subchunk-order

Conversation

@d-v-b
Copy link
Copy Markdown
Contributor

@d-v-b d-v-b commented May 26, 2026

This PR reverts the addition of random subchunk ordering in the sharding codec which was introduced in #3826. After this PR, choosing "unordered" will write chunks lexicographically but only as an implementation detail.

ilan-gold and others added 10 commits May 26, 2026 11:01
* feat: subchunk write order

* chore: export `SubchunkWriteOrder`

* chore: docs

* chore: relnote

* rename

* refactor: no enums

* Update docs/user-guide/performance.md

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>

* feat: deterministic but random order

* fix: make vectorized fetching less reliant on matching order

* chore: add hypothesis

* refactor: dead code

* refactor: more cleanup

* don't shard unless there is something to shard

* fix: dont mix chunk grid and sharding

---------

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
(cherry picked from commit 093a153)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hrough pickle

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_write_order

The hypothesis arrays() strategy passed both shards= and a ShardingCodec
serializer, which nested the codecs and left subchunk_write_order governing
only a 1-element inner grid. Drive sharding through the serializer alone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sted

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… FIXME

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ering

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves conflicts in the sharding codec, strategy, tests, docs, and changelog
by keeping this branch's rng-free implementation (unordered = no-promise) over
the incoming original feature commit (093a153), which reintroduced the rng.
Non-sharding incoming changes (string dtype pruning, metadata fixes) merged
cleanly and are kept.
@d-v-b d-v-b requested a review from ilan-gold May 26, 2026 11:53
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.53%. Comparing base (1cda981) to head (02781d2).

Files with missing lines Patch % Lines
src/zarr/codecs/sharding.py 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4011      +/-   ##
==========================================
- Coverage   93.55%   93.53%   -0.02%     
==========================================
  Files          88       88              
  Lines       11873    11871       -2     
==========================================
- Hits        11108    11104       -4     
- Misses        765      767       +2     
Files with missing lines Coverage Δ
src/zarr/testing/strategies.py 94.66% <100.00%> (ø)
src/zarr/codecs/sharding.py 91.52% <57.14%> (-0.61%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@d-v-b d-v-b requested a review from maxrjones May 27, 2026 15:42
@d-v-b d-v-b mentioned this pull request May 27, 2026
6 tasks
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.

2 participants