feat: subchunk write order

[ilan-gold]

In order to encourage ecosystem compatibility + reserve runtime setting strings/enums (see zarrs/zarrs-python#160), subchunk write order is expanded from morton to include lexicographic, colexicographic, and unordered (which is randomized).

TODO:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/user-guide/*.md
• Changes documented as a new file in changes/
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[d-v-b]

advantage of an enum over Literal["morton", "unordered", "lexicographic", "colexicographic"]?

[ilan-gold]

Just copied what was done for ShardingCodecIndexLocation!

[d-v-b]

I'm not a huge fan of enums in python (including ShardingCodecIndexingLocation), so unless you object I think it would be better to use a simple Literal + a final tuple of strings, like:

SubchunkWriteOrder = Literal["morton", "unordered", "lexicographic", "colexicographic"]
SUBCHUNK_WRITE_ORDER: Final[tuple[str, str, str, str]] = ("morton", "unordered", "lexicographic", "colexicographic")

[ilan-gold]

Done (hopefully)!

[ilan-gold]

cc @mkitti

Here and below, I don't think there is any need to construct the dict in morton order, right? There should be no correctness or performance hit here?

@d-v-b This now ensures we only shuffle in the unordered case once so the test is nice and clean - write once + get order, create a new codec with the same seed + create the iterator from that codec, match orders

[mkitti]

In Python, dicts are ordered and I think the optimal iteration order may need to be encoded in the dict the last time I examined the situation. I was just trying to preserve the situation before my edits.

[ilan-gold]

027c469

So this wasn't about dictionary order, but instead in the vectorized case, the order to ShardReader.to_dict_vectorized had to match that of what ShardReader was internally generating, as it turned out morton order. So I'm glad I caught this because I think it means the data was being corrupted for the other orders (which weren't getting hypothesis-tested).

So I'm going to add something to the hyptothesis tests for this.

I had the same feeling initially that the dictionary order mattered, but it turns out the final call to _encode_shard_dict actually handles the ordering for us to the output buffer while writing to the intermediate shard_dict can be done in any order, as long as the final buffer is done in the correct order

[codecov]

Codecov Report

❌ Patch coverage is 96.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.43%. Comparing base (5ca1690) to head (18d952e).

Files with missing lines	Patch %	Lines
src/zarr/codecs/sharding.py	93.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3826      +/-   ##
==========================================
+ Coverage   93.39%   93.43%   +0.04%     
==========================================
  Files          88       88              
  Lines       11839    11874      +35     
==========================================
+ Hits        11057    11095      +38     
+ Misses        782      779       -3     

Files with missing lines	Coverage Δ
src/zarr/codecs/__init__.py	100.00% <100.00%> (ø)
src/zarr/testing/strategies.py	94.66% <100.00%> (+1.77%)	⬆️
src/zarr/codecs/sharding.py	92.13% <93.33%> (+0.19%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[d-v-b]

this looks good, thanks!

we have some interesting follow-up work:

• finding a nice way to expose the sharding codec settings in the signature of create_array
• writing a chunk writing routine that can incrementally stream out a shard
• formalizing the distinction between codec attributes that get serialized to JSON and runtime-only attributes