diff --git a/changes/4526.bugfix.md b/changes/4526.bugfix.md new file mode 100644 index 0000000000..98a3281c98 --- /dev/null +++ b/changes/4526.bugfix.md @@ -0,0 +1 @@ +Array creation no longer fails when the codec chain cannot encode the data type's default scalar as the fill value, e.g. a `ScaleOffset` + `CastValue` chain mapping only a subrange of the dtype (`ShardingCodec` now probes its inner codec chain with the array's fill value instead of `dtype.default_scalar()`). If the array's fill value itself cannot be round-tripped through the codec chain, creation still fails: an unwritten chunk returns the fill value, so the array has to be readable before any chunks have been written. diff --git a/src/zarr/abc/codec.py b/src/zarr/abc/codec.py index c5e753d120..e565cc478c 100644 --- a/src/zarr/abc/codec.py +++ b/src/zarr/abc/codec.py @@ -168,6 +168,7 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: """Validates that the codec configuration is compatible with the array metadata. Raises errors when the codec configuration is not compatible. @@ -180,6 +181,12 @@ def validate( The array data type chunk_grid : ChunkGridMetadata The array chunk grid metadata + fill_value : TBaseScalar, optional + The array fill value. Codecs that probe the codec chain with a fill + value (e.g. ``ShardingCodec``) use this instead of the data type's + default scalar, so that the value an unwritten chunk would return is + the one validated. If ``None``, such codecs fall back to the data + type's default scalar. """ async def _decode_single(self, chunk_data: CO, chunk_spec: ArraySpec) -> CI: @@ -383,6 +390,7 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: """Validates that all codec configurations are compatible with the array metadata. Raises errors when a codec configuration is not compatible. @@ -395,6 +403,12 @@ def validate( The array data type chunk_grid : ChunkGridMetadata The array chunk grid metadata + fill_value : TBaseScalar, optional + The array fill value. Codecs that probe the codec chain with a fill + value (e.g. ``ShardingCodec``) use this instead of the data type's + default scalar, so that the value an unwritten chunk would return is + the one validated. If ``None``, such codecs fall back to the data + type's default scalar. """ ... diff --git a/src/zarr/codecs/cast_value.py b/src/zarr/codecs/cast_value.py index 629b4e7a27..bb3ed6d9b1 100644 --- a/src/zarr/codecs/cast_value.py +++ b/src/zarr/codecs/cast_value.py @@ -284,6 +284,7 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: # `dtype` is the source (the array's dtype); `self.dtype` is the # cast target. The spec requires both to be permitted, and rules diff --git a/src/zarr/codecs/scale_offset.py b/src/zarr/codecs/scale_offset.py index 2ec6015b6a..b1ba702ad1 100644 --- a/src/zarr/codecs/scale_offset.py +++ b/src/zarr/codecs/scale_offset.py @@ -383,6 +383,7 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: native = dtype.to_native_dtype() if not np.issubdtype(native, np.integer) and not np.issubdtype(native, np.floating): diff --git a/src/zarr/codecs/sharding.py b/src/zarr/codecs/sharding.py index d4ccf71cb9..82e671d258 100644 --- a/src/zarr/codecs/sharding.py +++ b/src/zarr/codecs/sharding.py @@ -619,6 +619,7 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: if len(self.chunk_shape) != len(shape): raise ValueError( @@ -642,18 +643,26 @@ def validate( f"Chunk edge length {edge} in dimension {i} is not " f"divisible by the shard's inner chunk size {inner}." ) - self._validate_inner_codecs(dtype) + self._validate_inner_codecs(dtype, fill_value) - def _validate_inner_codecs(self, dtype: ZDType[TBaseDType, TBaseScalar]) -> None: + def _validate_inner_codecs( + self, dtype: ZDType[TBaseDType, TBaseScalar], fill_value: TBaseScalar | None = None + ) -> None: """Validate the codecs that encode each inner chunk, as array metadata validates its codecs: each against the chunk the codecs before it leave it, a regular grid of one inner chunk. A sharding codec among them is thereby checked against the chunk it splits, which `validate` of the outer codec alone does not see. + + The codec chain is probed with the array's fill value, because that is the + value an unwritten chunk must return: the array has to be readable before any + chunks have been written. A fill value that cannot be round-tripped through + the inner codecs is therefore a genuine error and is raised here. If no fill + value is provided, the data type's default scalar is used. """ spec = ArraySpec( shape=self.chunk_shape, dtype=dtype, - fill_value=dtype.default_scalar(), + fill_value=dtype.default_scalar() if fill_value is None else fill_value, config=ArrayConfig.from_dict({}), prototype=default_buffer_prototype(), ) diff --git a/src/zarr/codecs/transpose.py b/src/zarr/codecs/transpose.py index 5756fba2b4..d885cfdfc1 100644 --- a/src/zarr/codecs/transpose.py +++ b/src/zarr/codecs/transpose.py @@ -52,6 +52,7 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: if len(self.order) != len(shape): raise ValueError( diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index 06f7338755..f59207ca80 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -4701,10 +4701,19 @@ async def init_array( sharding_codec = ShardingCodec( chunk_shape=inner_chunks_flat, codecs=sub_codecs, index_location=index_location ) + # Probe the inner codec chain with the array's fill value, since that + # is the value an unwritten chunk must return. Resolve the sentinel + # the same way `_create_metadata_v3` does, so validation and metadata + # agree on the fill value. + if fill_value is DEFAULT_FILL_VALUE or fill_value is None: + fill_value_probe: Any = zdtype.default_scalar() + else: + fill_value_probe = fill_value sharding_codec.validate( shape=inner_chunks_flat, dtype=zdtype, chunk_grid=grid, + fill_value=fill_value_probe, ) codecs_out = (sharding_codec,) else: diff --git a/src/zarr/core/codec_pipeline.py b/src/zarr/core/codec_pipeline.py index dd893ea8d8..ee56249cc0 100644 --- a/src/zarr/core/codec_pipeline.py +++ b/src/zarr/core/codec_pipeline.py @@ -657,9 +657,10 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: for codec in self: - codec.validate(shape=shape, dtype=dtype, chunk_grid=chunk_grid) + codec.validate(shape=shape, dtype=dtype, chunk_grid=chunk_grid, fill_value=fill_value) def compute_encoded_size(self, byte_length: int, array_spec: ArraySpec) -> int: for codec in self: @@ -1068,9 +1069,10 @@ def validate( shape: tuple[int, ...], dtype: ZDType[TBaseDType, TBaseScalar], chunk_grid: ChunkGridMetadata, + fill_value: TBaseScalar | None = None, ) -> None: for codec in self.codecs: - codec.validate(shape=shape, dtype=dtype, chunk_grid=chunk_grid) + codec.validate(shape=shape, dtype=dtype, chunk_grid=chunk_grid, fill_value=fill_value) def compute_encoded_size(self, byte_length: int, array_spec: ArraySpec) -> int: for codec in self: diff --git a/src/zarr/core/metadata/v3.py b/src/zarr/core/metadata/v3.py index 2d5d169e6b..b7fb7de547 100644 --- a/src/zarr/core/metadata/v3.py +++ b/src/zarr/core/metadata/v3.py @@ -571,7 +571,12 @@ def __init__( spec = array_spec for c in codecs_parsed_partial: evolved_codec = c.evolve_from_array_spec(spec) - evolved_codec.validate(shape=spec.shape, dtype=spec.dtype, chunk_grid=chunk_grid_parsed) + evolved_codec.validate( + shape=spec.shape, + dtype=spec.dtype, + chunk_grid=chunk_grid_parsed, + fill_value=spec.fill_value, + ) evolved.append(evolved_codec) spec = evolved_codec.resolve_metadata(spec) codecs_parsed = tuple(evolved) @@ -602,7 +607,12 @@ def _validate_metadata(self) -> None: if self.fill_value is None: raise ValueError("`fill_value` is required.") for codec in self.codecs: - codec.validate(shape=self.shape, dtype=self.data_type, chunk_grid=self.chunk_grid) + codec.validate( + shape=self.shape, + dtype=self.data_type, + chunk_grid=self.chunk_grid, + fill_value=self.fill_value, + ) @property def ndim(self) -> int: diff --git a/tests/test_codecs/test_cast_value.py b/tests/test_codecs/test_cast_value.py index c2e78770d9..c20f22c593 100644 --- a/tests/test_codecs/test_cast_value.py +++ b/tests/test_codecs/test_cast_value.py @@ -8,7 +8,7 @@ import zarr from tests.conftest import Expect, ExpectFail -from zarr.codecs import BytesCodec, TransposeCodec +from zarr.codecs import BytesCodec, ScaleOffset, TransposeCodec from zarr.codecs.cast_value import CastValue from zarr.storage import MemoryStore @@ -611,3 +611,79 @@ def make_array(filters: list[Any]) -> Any: array[:] = data assert_array_equal(array[:], data) + + +# --------------------------------------------------------------------------- +# Fill-value tolerance in resolve_metadata (issue #4511) +# --------------------------------------------------------------------------- + + +@requires_cast_value_rs +def test_creation_with_encodable_fill_value() -> None: + """A ScaleOffset+CastValue chain whose valid input range excludes the dtype's + default scalar (e.g. float32 values known to lie in [100, 200], scaled to + uint16) must not fail array creation when the array's fill value lies inside + that range: the fill value is the one probed against the codec chain, and it + round-trips (https://github.com/zarr-developers/zarr-python/issues/4511). + """ + minimum, maximum = 100.0, 200.0 + arr = zarr.create_array( + store=MemoryStore(), + shape=(4,), + dtype="float32", + fill_value=minimum, + chunks=(4,), + filters=[ + ScaleOffset(scale=65535 / (maximum - minimum), offset=minimum), + CastValue(data_type="uint16", rounding="nearest-even"), + ], + compressors=None, + ) + # Full-chunk writes contain no fill positions, so they encode cleanly. + data = np.array([120.0, 180.0, 100.0, 200.0], dtype="float32") + arr[:] = data + # Quantization to uint16 introduces a small rounding error. + np.testing.assert_allclose(np.asarray(arr[:]), data, atol=1e-2) + + +@requires_cast_value_rs +def test_creation_with_unencodable_fill_value_raises() -> None: + """An array whose fill value cannot be round-tripped through the codec chain + must fail creation: the array has to be readable before any chunks have been + written, and an unwritten chunk returns the fill value + (https://github.com/zarr-developers/zarr-python/issues/4511). + """ + minimum, maximum = 100.0, 200.0 + with pytest.raises(ValueError, match="out of range"): + zarr.create_array( + store=MemoryStore(), + shape=(4,), + dtype="float32", + # The default fill value (0.0) lies outside [100, 200]. + chunks=(4,), + filters=[ + ScaleOffset(scale=65535 / (maximum - minimum), offset=minimum), + CastValue(data_type="uint16", rounding="nearest-even"), + ], + compressors=None, + ) + + +@requires_cast_value_rs +def test_unencodable_fill_value_does_not_mask_invalid_config() -> None: + """Tolerating an unencodable fill value must not weaken validation: a codec + configuration that is structurally incompatible with the dtype still raises + at array creation time. + """ + with pytest.raises(ValueError, match="only supports integer and floating-point"): + zarr.create_array( + store=MemoryStore(), + shape=(4,), + dtype="str", + chunks=(4,), + filters=[ + ScaleOffset(scale=1, offset=100), + CastValue(data_type="uint16", rounding="nearest-even"), + ], + compressors=None, + ) diff --git a/tests/test_codecs/test_scale_offset.py b/tests/test_codecs/test_scale_offset.py index 0100d9fe80..a29518d844 100644 --- a/tests/test_codecs/test_scale_offset.py +++ b/tests/test_codecs/test_scale_offset.py @@ -488,3 +488,65 @@ def test_compute_encoded_size() -> None: # The chunk_spec argument is unused; pass any sentinel assert codec.compute_encoded_size(input_byte_length=100, _chunk_spec=None) == 100 # type: ignore[arg-type] assert codec.compute_encoded_size(input_byte_length=0, _chunk_spec=None) == 0 # type: ignore[arg-type] + + +# --------------------------------------------------------------------------- +# Fill-value tolerance in resolve_metadata (issue #4511) +# --------------------------------------------------------------------------- + + +def test_creation_with_encodable_fill_value() -> None: + """A scale/offset pair mapping only a subrange of the dtype makes the dtype's + default scalar (0) unencodable, but array creation succeeds when the array's + fill value lies inside that subrange: the fill value is the one probed + against the codec chain, and it round-trips + (https://github.com/zarr-developers/zarr-python/issues/4511). + """ + arr = zarr.create_array( + store=MemoryStore(), + shape=(4,), + dtype="uint16", + fill_value=100, + chunks=(4,), + filters=[ScaleOffset(offset=100, scale=1)], + compressors=None, + ) + # Full-chunk writes contain no fill positions, so they encode cleanly. + # Encoded values must stay within uint16 after decode adds the offset back. + data = np.array([100, 150, 200, 65435], dtype="uint16") + arr[:] = data + np.testing.assert_array_equal(arr[:], data) + + +def test_creation_with_unencodable_fill_value_raises() -> None: + """An array whose fill value cannot be round-tripped through the codec chain + must fail creation: the array has to be readable before any chunks have been + written, and an unwritten chunk returns the fill value + (https://github.com/zarr-developers/zarr-python/issues/4511). + """ + with pytest.raises(ValueError, match="outside the range"): + zarr.create_array( + store=MemoryStore(), + shape=(4,), + dtype="uint16", + # The default fill value (0) lies outside [100, 65535]. + chunks=(4,), + filters=[ScaleOffset(offset=100, scale=1)], + compressors=None, + ) + + +def test_unencodable_fill_value_does_not_mask_invalid_config() -> None: + """Tolerating an unencodable fill value must not weaken validation: a codec + configuration that is structurally incompatible with the dtype still raises + at array creation time. + """ + with pytest.raises(ValueError, match="only supports integer and floating-point"): + zarr.create_array( + store=MemoryStore(), + shape=(4,), + dtype="str", + chunks=(4,), + filters=[ScaleOffset(offset=100, scale=1)], + compressors=None, + ) diff --git a/tests/test_codecs/test_sharding.py b/tests/test_codecs/test_sharding.py index 5712c9576d..bee94405fd 100644 --- a/tests/test_codecs/test_sharding.py +++ b/tests/test_codecs/test_sharding.py @@ -19,6 +19,7 @@ BytesCodec, Crc32cCodec, GzipCodec, + ScaleOffset, ShardingCodec, TransposeCodec, ) @@ -1522,3 +1523,50 @@ def test_open_rejects_stored_nested_sharding_with_indivisible_inner_chunk_shape( doc["codecs"][0]["configuration"]["codecs"][0]["configuration"]["chunk_shape"] = (4, 7) with pytest.raises(ValueError, match="not divisible"): ArrayV3Metadata.from_dict(doc) + + +# --------------------------------------------------------------------------- +# Inner-codec validation tolerance for unencodable fill values (issue #4511) +# --------------------------------------------------------------------------- + + +def test_create_with_encodable_fill_value_inner_codecs() -> None: + """`ShardingCodec._validate_inner_codecs` probes the inner codec chain with + the array's fill value. When the chain maps only a subrange of the dtype + (e.g. a scale/offset pair for values in [100, 200]) but the fill value lies + inside that subrange, the configuration is valid and array creation succeeds + (https://github.com/zarr-developers/zarr-python/issues/4511). + """ + arr = zarr.create_array( + store=MemoryStore(), + shape=(16,), + chunks=(4,), + shards=(16,), + dtype="uint16", + fill_value=100, + filters=[ScaleOffset(offset=100, scale=1)], + ) + # Full-shard writes contain no fill positions, so they encode cleanly. + # Encoded values must stay within uint16 after decode adds the offset back. + data = np.array([100, 150, 200, 65435] * 4, dtype="uint16") + arr[:] = data + np.testing.assert_array_equal(arr[:], data) + + +def test_create_with_unencodable_fill_value_inner_codecs_raises() -> None: + """`ShardingCodec._validate_inner_codecs` must raise when the array's fill + value cannot be round-tripped through the inner codec chain: an unwritten + shard returns the fill value, so the array has to be readable before any + chunks have been written + (https://github.com/zarr-developers/zarr-python/issues/4511). + """ + with pytest.raises(ValueError, match="outside the range"): + zarr.create_array( + store=MemoryStore(), + shape=(16,), + chunks=(4,), + shards=(16,), + dtype="uint16", + # The default fill value (0) lies outside [100, 65535]. + filters=[ScaleOffset(offset=100, scale=1)], + )