Repository navigation
fix: tolerate unencodable default_scalar in ShardingCodec.validate - #4526
Open
inchang-ing wants to merge 3 commits into
Open
inchang-ing wants to merge 3 commits into
inchang-ing wants to merge 3 commits into
Conversation
ShardingCodec._validate_inner_codecs (and ArrayV3Metadata codec threading) probe the codec chain with dtype.default_scalar() as the fill value. When the chain maps only a subrange of the dtype — e.g. ScaleOffset + CastValue scaling float32 values in [100, 200] to uint16 — the probe value itself is unencodable, and array creation failed even though no real value ever needs that encoding. Make the fill-value threading in ScaleOffset.resolve_metadata and CastValue.resolve_metadata best-effort: if encoding the fill value raises a value-domain error (ValueError/OverflowError), carry the fill value forward unchanged instead of failing. Shape/dtype threading and per-codec validate() are untouched, so structurally invalid configurations (wrong dtype for a codec, scale=0, out_of_range='wrap' with a float target, ...) still raise at creation time. Genuine value-domain errors for actual chunk data still surface in _encode_sync. Fixes zarr-developers#4511
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4526 +/- ##
=======================================
Coverage 94.71% 94.71%
=======================================
Files 94 94
Lines 13663 13669 +6
=======================================
+ Hits 12941 12947 +6
Misses 722 722
🚀 New features to boost your workflow:
|
This branch has not been deployed
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
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.
Fixes #4511
Root cause
When creating an array, codec chains are validated by threading an
ArraySpecthrough each codec'sresolve_metadata:ArrayV3Metadata.__init__threads the array's fill value (which defaults todtype.default_scalar()) through the codec chain,ShardingCodec._validate_inner_codecsthreads the inner codec chain withdtype.default_scalar()as the probe fill value.Value-transforming codecs (
ScaleOffset,CastValue) encode the fill value as part of that threading. When the chain maps only a subrange of the dtype — e.g. float32 values known to lie in [100, 200], scaled to [0, 65535] and cast to uint16 — the probe value (0) scales to -65535, whichCastValuerejects:Array creation failed even though no real value ever needs that encoding: the failure is a property of the probe fill value, not of the codec configuration.
Fix
Make the fill-value threading best-effort in
ScaleOffset.resolve_metadataandCastValue.resolve_metadata: if encoding the fill value raises a value-domain error (ValueError/OverflowError), carry the fill value forward unchanged instead of failing. Shape/dtype threading and per-codecvalidate()are untouched, so:scale=0,out_of_range='wrap'with a float target, ...) — verified below;_encode_syncat write time (writing a value outside [100, 200] with this chain still errors, as before).The same tolerance also fixes the runtime pipeline-evolution path, which threaded the same unencodable fill through
resolve_metadata.Changes
src/zarr/codecs/scale_offset.py: tolerate value-domain failures when encoding the fill value inresolve_metadatasrc/zarr/codecs/cast_value.py: sametests/test_codecs/test_scale_offset.py: creation succeeds with an unencodable default fill value (uint16 +ScaleOffset(offset=100)); invalid config still raisestests/test_codecs/test_cast_value.py: the issue's exact scenario (float32 [100, 200] → uint16 via ScaleOffset+CastValue) creates and round-trips; invalid config still raisestests/test_codecs/test_sharding.py: sharded array creation succeeds when the inner codec chain cannot encodedefault_scalarVerification
main(v3.4.2.dev14): the issue's repro script raisedValueError: Value -65535 out of range [0, 65535].pytest tests/test_codecs/ tests/test_metadata/test_v3.py: 757 passed, 1 skipped, 2 xfailed.pytest tests/test_array.py tests/test_codec_pipeline.py tests/test_unified_chunk_grid.py: 2737 passed, 756 skipped.ScaleOffseton a string dtype,out_of_range='wrap'with a float target, andscale=0all raiseValueErrorat creation.