fix: resolve chunks=-1 to chunk size 1 on zero-length axes - #315
Open
d-v-b wants to merge 5 commits into
Open
Conversation
… style pass - #310 (zarr-developers#4296) * docs: clarify gzip MTIME handling in comparison helper and fused-pipeline test docs Follow-up to zarr-developers#4270: document that _gzip_streams_equal_except_mtime skips bytes 4-8 (the RFC 1952 MTIME field of the standard 10-byte gzip header), and update the AsyncChunkTransform section comment and test docstring to note that byte-identity is expected except for gzip's MTIME header field. Assisted-by: ClaudeCode:claude-fable-5 * test: style pass on fused-pipeline tests Hoist the imports that nearly every test re-imported locally, add a shared _make_spec helper to replace nine copies of the ArraySpec boilerplate, and convert test_async_chunk_transform_matches_sync to Expect cases where each case declares its expected byte-comparison function (exact equality, or gzip-MTIME-tolerant), removing the isinstance branch from the test body. Also add the missing read-back assertion in test_sync_write_async_read_roundtrip, which previously read into a buffer and never compared it to the written data. Assisted-by: ClaudeCode:claude-fable-5 * test: require a ZDType in _make_spec instead of a dtype string Assisted-by: ClaudeCode:claude-fable-5
…opers#4277) * feat: name packages that provide a codec zarr cannot find Closes zarr-developers#4271. When zarr fails to resolve a codec it now says which Python packages are known to provide it, instead of raising a bare KeyError holding only the codec name: An implementation for codec 'wavpack' is not available. Register one explicitly using the codec registry (see <docs>), or install a Python package that registers a codec implementation with numcodecs. Known packages supporting this codec: wavpack-numcodecs. Two hand-maintained tables in zarr/registry.py hold the mapping, one per Zarr format, because the two formats resolve codecs through different registries and the same name can mean different things in each: `imagecodecs_*` names are declared by `virtual-tiff` under the `zarr.codecs` entry point group and by `imagecodecs-numcodecs` under `numcodecs.codecs`, and `crc32c` is a codec zarr implements itself in format 3 while in format 2 it needs `numcodecs[crc32c]`. Each table has an exact-match and a prefix-match half, since packages that provide many codecs namespace them behind a shared prefix. Entries cover third-party packages and the codecs numcodecs gates behind its own optional dependencies -- `zfpy`, `pcodec`, `crc32c` and `msgpack2` -- which are the most common missing-codec case in practice. Backwards compatibility: `get_codec_class` now raises `zarr.errors.UnknownCodecError` instead of `KeyError`, both for a codec with no registered implementation and for a codec whose configured implementation is not registered. `get_numcodec` raises it instead of the ValueError numcodecs raises for an unregistered format 2 codec id. All are subclasses of `ValueError`. Carrying the message on a `KeyError` was not an option: `KeyError.__str__` reprs its argument, so a multi-sentence message comes back quoted and escaped. `UnknownCodecError` is now exported from `zarr.errors`, since users are being told to catch it. `get_numcodec` supports numcodecs down to the declared 0.14 floor: `numcodecs.errors` only exists from 0.15.1, so the unregistered-codec check prefers that exception type where it is importable and falls back to matching the message otherwise. Signed-off-by: arcusbuilds <srijankeshri007@gmail.com> * fix: address review feedback 1. parse_codecs converts KeyError from from_dict again. The removed try/except wrapped the whole expression, not just the registry lookup, so a codec whose from_dict indexes a malformed configuration leaked a bare KeyError out of metadata parsing. On the zarr.open fallback path that KeyError was swallowed and reported as an unrelated group error: with mode="a" it surfaced as `TypeError: open_group() got an unexpected keyword argument 'shape'`. The catch is narrow, around from_dict only, since get_codec_class now raises for the lookup half. It raises MetadataValidationError naming the codec and the missing key rather than restoring the old message, which reported the missing configuration key as though it were the codec name ("Unknown codec: 'required_option'"). 2. The config-pin branch raises BadConfigError, matching get_pipeline_class, get_buffer_class and get_ndbuffer_class, which all use it for this exact situation. This also stops migrate_v3._find_numcodecs_zarr3 misreporting a config typo as a missing numcodecs codec. 3. Three tests assumed the advertised packages were absent. Both registries are entry-point driven, so they failed in any environment with zarr-n5 or wavpack-numcodecs installed, which are the packages the messages recommend. Two fixtures now remove the specific entry for the duration of the test. Verified by installing both packages and re-running. 4. test_mapping_does_not_shadow_builtin_codecs selected on "registry is non-empty", conflating loaded-in-this-process with implemented-by-zarr. It now selects on the implementing class's module, so a lazy-loaded third-party codec cannot fail it. 5. get_numcodec's Raises section notes that numcodecs' own error propagates unchanged when data carries no string "id". 6. Dropped the `pragma: no cover` on the numcodecs < 0.15.1 fallback. The min_deps env pins numcodecs==0.14.* and runs run-coverage, so that branch is measured. Also hoisted the repeated in-function imports in tests/test_registry.py to the module level. Signed-off-by: arcusbuilds <srijankeshri007@gmail.com> * fix: address the second review round Four defects, all found by review after the previous round was reported clean. get_numcodec no longer wraps the numcodecs call in an exception handler. Catching cannot distinguish "this id is unregistered" from "a registered codec rejected its configuration" or "a wrapper codec failed to resolve an inner codec", and it was relabelling both of the latter with the outer id plus a package hint that was wrong. Reproduced: a wrapper registered as `wavpack` whose from_config resolved a missing inner codec reported "An implementation for codec 'wavpack' is not available ... install wavpack-numcodecs", when wavpack was installed and the missing codec was something else entirely. It now performs the lookups numcodecs performs, before delegating. That also fixes reading `id` off a non-mapping input, which raised AttributeError where a ValueError used to propagate. And it removes _is_missing_numcodec_error, and with it the numcodecs <0.15.1 compatibility branch, since there is no longer an exception to classify. Note this narrows the zarr error to Mapping inputs; a duck-typed mapping now gets numcodecs' error instead, as it did before this PR. The imagecodecs_ prefix in the Zarr format 3 table pointed at virtual-tiff, which declares 15 of the 81 imagecodecs_* names under zarr.codecs; imagecodecs-numcodecs declares all 81, but under numcodecs.codecs. Users of the other 66 names were told to install a package that does not provide them. The format 3 side now lists the 15 exact names, so the rest get no hint rather than a wrong one. The format 2 side keeps the prefix, where it is correct. test_parse_codecs_converts_keyerror_from_from_dict leaked test_picky into the global codec registry, in the file that also reads that global. Tests: mutation testing showed six mutations surviving. Added the missing coverage for a registered codec rejecting its configuration, wrapper codecs resolving an inner codec by either route, non-mapping input, the imagecodecs_ over-match, and the _resolve_codec entry point. Message assertions now pin the whole string and the URL constants rather than substrings, which had allowed both documentation URLs to be replaced with wrong ones and half the message body to be deleted with every test still passing. Also corrects the docs path to src/zarr/registry.py, and rewrites the changelog to lead with the exception-type changes and to document the parse_codecs change it had omitted. Signed-off-by: arcusbuilds <srijankeshri007@gmail.com> --------- Signed-off-by: arcusbuilds <srijankeshri007@gmail.com> Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
…al arrays (zarr-developers#4305) * Fix infinite loop in shard size guessing for 0-dimensional arrays _guess_num_chunks_per_axis_shard looped forever for rank-0 arrays because neither loop condition depends on the counter when chunk_shape is empty. Return 1 immediately in that case, which makes the budget path produce shards=() just like the no-budget path. Closes zarr-developers#4304 * Add changelog fragment
7 tasks
The -1 chunk sentinel resolved to chunk size 0 on zero-length axes, which broke every downstream sharding path differently: a ValueError with shards=None, a ZeroDivisionError with shards="auto", and an infinite loop with shards="auto" plus array.target_shard_size_bytes. Clamp it to 1, matching the auto-chunking clamp in _guess_regular_chunks. Also guard _guess_num_chunks_per_axis_shard against zero-size chunk shapes directly (same non-terminating-loop cause as the 0-d case fixed in zarr-developers#4305), and fix the arithmetic in its docstring example. Follow-up to zarr-developers#4305 / issue zarr-developers#4304. Assisted-by: ClaudeCode:claude-fable-5-1
Assisted-by: ClaudeCode:claude-fable-5-1
d-v-b
force-pushed
the
fix/shard-guess-zero-size-chunks
branch
from
September 2, 2026 12:01
873e1df to
f850df5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖
Follow-up to zarr-developers#4305 (branched from its head, so its two commits appear here until it merges upstream and the fork syncs).
Reviewing zarr-developers#4305 surfaced that the
num_axes == 0guard fixes the reported 0-d hang but leaves the same non-terminating loop reachable through the public API:chunks=-1on a zero-length axis resolves to chunk size 0 innormalize_chunks_1d, and a zerobytes_per_chunkmakes both loop conditions in_guess_num_chunks_per_axis_shardpermanently true. Before this change:zarr.create_array(store={}, shape=(0,), dtype='int64', chunks=-1, shards=None)→ValueErrorshards='auto'→ZeroDivisionErrorshards='auto'+array.target_shard_size_bytesset → infinite loopChanges:
normalize_chunks_1d: the-1sentinel now clamps to chunk size 1 on a zero-length span, matching thenp.maximum(shape, 1)clamp in_guess_regular_chunks. All three cases above now succeed withchunks=(1,), consistent withchunks='auto'._guess_num_chunks_per_axis_shard: defensively guardbytes_per_chunk == 0(the function is unit-tested directly, so degenerate inputs remain reachable), in the spirit of the guard added to_guess_chunksin fix: auto-chunking when auto-sharding 1MiB number zarr-developers/zarr-python#3603.returned_val ** len(chunk_shape), notreturned_val * len(chunk_shape)) and documented the degenerate-input behavior.-1-on-empty-span case fornormalize_chunks_1d, parametrized degenerate shapes for the helper, and an end-to-endshape=(0,)+chunks=-1+shards='auto'test with and without a shard size budget.🤖 Generated with Claude Code