Skip to content

Commit e2a4e7b

Browse files
committed
refactor(zarr-metadata): draw a pipeline only through pipelines_for
Fold the check that a pipeline can be drawn into the function that draws it, so no caller can skip it, and rename the shared drawing module to `_draw`. `fill_values_of` raises `LookupError` for a field no definition in the strategies reads, the not-real-number data types are defined once in `_arithmetic`, and the tests read pipelines for partly known chunks and spell the cast targets out rather than reusing the code's own constants. Assisted-by: ClaudeCode:claude-opus-5-5
1 parent cba89b4 commit e2a4e7b

7 files changed

Lines changed: 102 additions & 64 deletions

File tree

‎packages/zarr-metadata/src/zarr_metadata/testing/_codec.py‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,21 @@
1616
from functools import partial
1717
from typing import TYPE_CHECKING, Any, Final
1818

19+
from hypothesis import reject
1920
from hypothesis import strategies as st
2021

2122
from zarr_metadata.testing._data_type import FLOAT_WIDTHS, INTEGER_RANGES, finite_floats
22-
from zarr_metadata.testing._nested import (
23+
from zarr_metadata.testing._draw import (
2324
briefly,
2425
data_type_fields,
2526
data_types,
2627
fill_values_of,
27-
pipeline_walk,
28+
pipelines_for,
2829
read,
29-
takes,
3030
)
3131
from zarr_metadata.testing._strategy import CodecStrategy
3232
from zarr_metadata.v3._definition import Chunk, DataTypeDefinition, storage_of
33-
from zarr_metadata.v3.codec._arithmetic import COMPLEX, FLOATING_POINT, NOT_NUMBERS, read_name
33+
from zarr_metadata.v3.codec._arithmetic import FLOATING_POINT, NO_REAL_NUMBERS, read_name
3434
from zarr_metadata.v3.codec.blosc import BLOSC_CNAME, BLOSC_CODEC, BLOSC_NO_SHUFFLE, BLOSC_SHUFFLE
3535
from zarr_metadata.v3.codec.bytes import BYTES_CODEC, ENDIANNESS
3636
from zarr_metadata.v3.codec.cast_value import (
@@ -134,8 +134,8 @@ def _scale_offset(chunk: Chunk, _strategies: Strategies) -> Configurations | Non
134134
are small non-negative integers, which every listed type holds.
135135
"""
136136
data_type = chunk.data_type
137-
read = data_type is not None and data_type.resolution == "read"
138-
definition = data_type.definition if data_type is not None and read else None
137+
is_read = data_type is not None and data_type.resolution == "read"
138+
definition = data_type.definition if data_type is not None and is_read else None
139139
if definition is None:
140140
offset: st.SearchStrategy[JSONValue] = _SMALL_NUMBERS
141141
scale: st.SearchStrategy[JSONValue] = st.integers(1, 127)
@@ -156,10 +156,6 @@ def _unchunked(configurations: Configurations) -> CodecStrategy:
156156
return CodecStrategy(configurations=lambda _chunk, _strategies: configurations)
157157

158158

159-
_NO_REAL_NUMBERS: Final = NOT_NUMBERS | COMPLEX
160-
"""The data types `cast_value` takes neither from nor to: none of them models real numbers."""
161-
162-
163159
def _source_scalars(chunk: Chunk, strategies: Strategies) -> st.SearchStrategy[JSONValue]:
164160
"""Values of the chunk's data type: its fill values, or small numbers when no scope read it or no strategy draws for it."""
165161
source = chunk.data_type
@@ -203,9 +199,9 @@ def _cast_value(chunk: Chunk, strategies: Strategies) -> Configurations | None:
203199
of its direction; `wrap` is drawn only for a type that is not
204200
floating-point.
205201
"""
206-
if read_name(chunk.data_type) in _NO_REAL_NUMBERS:
202+
if read_name(chunk.data_type) in NO_REAL_NUMBERS:
207203
return None
208-
targets = [d for d in data_types(strategies) if d.name not in _NO_REAL_NUMBERS]
204+
targets = [d for d in data_types(strategies) if d.name not in NO_REAL_NUMBERS]
209205
if len(targets) == 0:
210206
return None
211207
return _cast_value_configuration(targets, _source_scalars(chunk, strategies), strategies)
@@ -244,10 +240,14 @@ def _sharding_configuration(
244240
"index_codecs": (),
245241
}
246242
handed = SHARDING_INDEXED_CODEC.pipelines(sized, {}, chunk)
243+
codecs = pipelines_for(handed["codecs"], inner)
244+
index_codecs = pipelines_for(handed["index_codecs"], strategies, _STATIC)
245+
if codecs is None or index_codecs is None:
246+
reject() # a codec that takes a chunk of the data type, but not of these lengths
247247
configuration: dict[str, st.SearchStrategy[Any]] = {
248248
"chunk_shape": st.just(chunk_shape),
249-
"codecs": pipeline_walk(handed["codecs"], inner),
250-
"index_codecs": pipeline_walk(handed["index_codecs"], strategies, _STATIC),
249+
"codecs": codecs,
250+
"index_codecs": index_codecs,
251251
}
252252
optional = {"index_location": st.sampled_from(SHARDING_INDEX_LOCATION)}
253253
return draw(st.fixed_dictionaries(configuration, optional=optional))
@@ -273,9 +273,9 @@ def _sharding(chunk: Chunk, strategies: Strategies, *, depth: int) -> Configurat
273273
not drawn: shards drawn without end would nest without end.
274274
"""
275275
handed = SHARDING_INDEXED_CODEC.pipelines(_NO_SHAPE, {}, Chunk(None, chunk.data_type))
276-
if not takes(Chunk(None, handed["codecs"].data_type), strategies):
276+
if pipelines_for(Chunk(None, handed["codecs"].data_type), strategies) is None:
277277
return None
278-
if not takes(Chunk(None, handed["index_codecs"].data_type), strategies, _STATIC):
278+
if pipelines_for(Chunk(None, handed["index_codecs"].data_type), strategies, _STATIC) is None:
279279
return None
280280
inner = {d: s for d, s in strategies.items() if d is not SHARDING_INDEXED_CODEC}
281281
if depth > 0:

‎packages/zarr-metadata/src/zarr_metadata/testing/_data_type.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from hypothesis import strategies as st
1717

18-
from zarr_metadata.testing._nested import (
18+
from zarr_metadata.testing._draw import (
1919
briefly,
2020
data_type_fields,
2121
data_types,

packages/zarr-metadata/src/zarr_metadata/testing/_nested.py renamed to packages/zarr-metadata/src/zarr_metadata/testing/_draw.py

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,15 @@ def scope_of(strategies: Strategies) -> Context:
117117

118118

119119
def read(field: object, kind: type[D], strategies: Strategies) -> Resolved[D]:
120-
"""`field`, drawn from `strategies`, read in their scope; a `ValueError` if it is refused.
120+
"""`field`, drawn from `strategies` or held by a configuration handed to one, read in their scope; a `ValueError` if it is refused.
121121
122-
A strategy draws what its definition accepts, so a refused draw is a
123-
fault in the strategy, not something to draw again.
122+
A strategy draws what its definition accepts, so a refused field is a
123+
fault in a strategy or in the configuration it was handed, not
124+
something to draw again.
124125
"""
125126
resolved, problems = resolve(field, kind, scope_of(strategies))
126127
if len(problems) != 0:
127-
msg = f"a strategy drew {field!r}, which is refused: {problems!r}"
128+
msg = f"{field!r}, read in the strategies' scope, is refused: {problems!r}"
128129
raise ValueError(msg)
129130
return resolved
130131

@@ -190,10 +191,11 @@ def data_type_fields(
190191
def fill_values_of(
191192
data_type: Resolved[DataTypeDefinition[Any]], strategies: Strategies
192193
) -> SearchStrategy[JSONValue]:
193-
"""Fill values of `data_type`, a data type field read in the strategies' scope."""
194+
"""Fill values of `data_type`, a data type field read in the strategies' scope; a `LookupError` if no definition in it read the field."""
194195
definition, configuration = data_type.definition, data_type.configuration
195-
assert definition is not None
196-
assert configuration is not None
196+
if definition is None or configuration is None:
197+
msg = f"no strategy draws fill values of {data_type.json!r}, which no definition in `strategies` reads"
198+
raise LookupError(msg)
197199
found = cast("DataTypeStrategy", strategy_for(definition, strategies))
198200
return found.fill_values(configuration, strategies)
199201

@@ -228,12 +230,18 @@ def _holds_pipelines(definition: CodecDefinition[Any]) -> bool:
228230
return definition.pipelines is not no_pipelines
229231

230232

231-
def takes(chunk: Chunk, strategies: Strategies, sizes: frozenset[CodecSize] = EVERY_SIZE) -> bool:
232-
"""Whether some array -> bytes codec that holds no pipelines takes `chunk` as it is.
233+
def pipelines_for(
234+
chunk: Chunk, strategies: Strategies, sizes: frozenset[CodecSize] = EVERY_SIZE
235+
) -> SearchStrategy[tuple[ZarrV3MetadataFieldJSON, ...]] | None:
236+
"""Pipelines handed `chunk`, of codecs of one of `sizes`; None when no array -> bytes codec that holds no pipelines takes the chunk as it is.
233237
234-
Enough for a pipeline to be drawn for it: one of only that codec.
238+
One that does is enough for a pipeline: one of only that codec. One
239+
that holds pipelines of its own takes a chunk only if what it holds
240+
does, so it settles nothing.
235241
"""
236-
return len(_taking(chunk, "array_bytes", sizes, strategies, holding=False)) != 0
242+
if len(_taking(chunk, "array_bytes", sizes, strategies, holding=False)) == 0:
243+
return None
244+
return _walk(chunk, strategies, sizes)
237245

238246

239247
@st.composite
@@ -245,19 +253,15 @@ def _codec(
245253

246254

247255
@st.composite
248-
def pipeline_walk(
249-
draw: st.DrawFn,
250-
chunk: Chunk,
251-
strategies: Strategies,
252-
sizes: frozenset[CodecSize] = EVERY_SIZE,
256+
def _walk(
257+
draw: st.DrawFn, chunk: Chunk, strategies: Strategies, sizes: frozenset[CodecSize]
253258
) -> tuple[ZarrV3MetadataFieldJSON, ...]:
254259
"""Codecs for a pipeline handed `chunk`, each drawn for the chunk the one before it hands on.
255260
256261
Array -> array codecs, then one array -> bytes codec, then bytes ->
257262
bytes codecs, each of one of `sizes`. What an array -> array codec
258263
hands on is what its definition's `transition` says of the codec as
259264
drawn: a `cast_value` hands the next codec the data type it casts to.
260-
Draw it only for a chunk `takes` says a pipeline is drawn for.
261265
"""
262266
codecs: list[ZarrV3MetadataFieldJSON] = []
263267
handed = chunk
@@ -288,9 +292,8 @@ def pipeline_walk(
288292
"data_type_fields",
289293
"data_types",
290294
"fill_values_of",
291-
"pipeline_walk",
295+
"pipelines_for",
292296
"read",
293297
"scope_of",
294298
"strategy_for",
295-
"takes",
296299
]

‎packages/zarr-metadata/src/zarr_metadata/testing/strategies.py‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,12 @@ def test_my_reader_reads_every_blosc_codec(field):
5050
from zarr_metadata.testing._chunk_key_encoding import CHUNK_KEY_ENCODING_STRATEGIES
5151
from zarr_metadata.testing._codec import CODEC_STRATEGIES
5252
from zarr_metadata.testing._data_type import DATA_TYPE_STRATEGIES
53-
from zarr_metadata.testing._nested import (
53+
from zarr_metadata.testing._draw import (
5454
any_spelling,
5555
briefly,
56-
pipeline_walk,
56+
pipelines_for,
5757
scope_of,
5858
strategy_for,
59-
takes,
6059
)
6160
from zarr_metadata.testing._strategy import (
6261
NOTHING_KNOWN,
@@ -190,9 +189,7 @@ def pipelines(
190189
defines takes a `string` one.
191190
"""
192191
strategies = briefly(strategies)
193-
if not takes(chunk, strategies):
194-
return None
195-
return pipeline_walk(chunk, strategies)
192+
return pipelines_for(chunk, strategies)
196193

197194

198195
def _canonical(

‎packages/zarr-metadata/src/zarr_metadata/v3/codec/_arithmetic.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,15 @@
6767
"""
6868

6969

70+
NO_REAL_NUMBERS: Final = NOT_NUMBERS | COMPLEX
71+
"""The data types that model no real number, which `cast_value` takes neither from nor to."""
72+
73+
7074
def read_name(data_type: Resolved[Any] | None) -> str | None:
7175
"""The name the definition that read `data_type` is filed under; None when no definition read it."""
7276
if data_type is None or data_type.resolution != "read" or data_type.definition is None:
7377
return None
7478
return data_type.definition.name
7579

7680

77-
__all__ = ["COMPLEX", "FLOATING_POINT", "NOT_NUMBERS", "read_name"]
81+
__all__ = ["COMPLEX", "FLOATING_POINT", "NOT_NUMBERS", "NO_REAL_NUMBERS", "read_name"]

‎packages/zarr-metadata/src/zarr_metadata/v3/codec/cast_value.py‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,7 @@
1919
fill_value_problems,
2020
named_configuration,
2121
)
22-
from zarr_metadata.v3.codec._arithmetic import (
23-
COMPLEX,
24-
FLOATING_POINT,
25-
NOT_NUMBERS,
26-
read_name,
27-
)
22+
from zarr_metadata.v3.codec._arithmetic import FLOATING_POINT, NO_REAL_NUMBERS, read_name
2823

2924
CAST_VALUE_CODEC_NAME: Final = "cast_value"
3025
"""The `name` field value of the `cast_value` codec."""
@@ -111,10 +106,6 @@ class CastValueCodecObject(TypedDict, closed=True):
111106
"""
112107

113108

114-
_NO_REAL_NUMBERS: Final = NOT_NUMBERS | COMPLEX
115-
"""The data types the codec takes neither from nor to: none of them models real numbers."""
116-
117-
118109
def _scalars(
119110
configuration: CastValueCodecConfiguration, side: Literal["source", "target"]
120111
) -> Iterator[tuple[tuple[str | int, ...], JSONValue]]:
@@ -157,7 +148,7 @@ def _rules(
157148
if target is None or name is None:
158149
return
159150
written, _, _ = named_configuration(target.json)
160-
if name in _NO_REAL_NUMBERS:
151+
if name in NO_REAL_NUMBERS:
161152
yield ValidationProblem(
162153
("data_type",),
163154
f"expected a data type that models real numbers, got {written!r}",
@@ -188,7 +179,7 @@ def _chunk_rules(
188179
name = read_name(source)
189180
if source is None or name is None:
190181
return
191-
if name in _NO_REAL_NUMBERS:
182+
if name in NO_REAL_NUMBERS:
192183
written, _, _ = named_configuration(source.json)
193184
yield ValidationProblem(
194185
(),

‎packages/zarr-metadata/tests/testing/test_strategies.py‎

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
from zarr_metadata.v3._definition import as_kind, kind_of, no_pipelines, storage_of, unchanged
4040
from zarr_metadata.v3._pipeline import read_pipeline
4141
from zarr_metadata.v3.chunk_grid.rectilinear import RECTILINEAR_CHUNK_GRID
42-
from zarr_metadata.v3.codec._arithmetic import COMPLEX, NOT_NUMBERS
4342
from zarr_metadata.v3.codec.bytes import BYTES_CODEC
4443
from zarr_metadata.v3.codec.cast_value import CAST_VALUE_CODEC
4544
from zarr_metadata.v3.codec.gzip import GZIP_CODEC
@@ -276,7 +275,28 @@ def test_a_cast_from_a_data_type_without_a_strategy_draws_numbers() -> None:
276275
find(configurations, lambda configuration: "scalar_map" in configuration, settings=_REACH)
277276

278277

279-
@given(data=st.data(), chunk=_chunks())
278+
@st.composite
279+
def _partly_known_chunks(draw: st.DrawFn) -> Chunk:
280+
"""A chunk some of which nothing says: its lengths along an axis or at all, or its data type, read by nothing in scope."""
281+
chunk = draw(_chunks())
282+
lengths = draw(
283+
st.sampled_from(
284+
[
285+
chunk.lengths,
286+
None,
287+
None if chunk.lengths is None else tuple(None for _ in chunk.lengths),
288+
]
289+
)
290+
)
291+
data_type = draw(st.sampled_from([chunk.data_type, None, _UNREAD_DATA_TYPE]))
292+
return Chunk(lengths, data_type)
293+
294+
295+
_UNREAD_DATA_TYPE = resolve({"name": "an.unread.type"}, DataTypeDefinition, CORE_AND_EXTENSIONS)[0]
296+
"""A data type field that nothing in scope claims: a chunk of it says nothing of its values."""
297+
298+
299+
@given(data=st.data(), chunk=_chunks() | _partly_known_chunks())
280300
def test_pipelines_read(data: st.DataObject, chunk: Chunk) -> None:
281301
"""A pipeline drawn for a chunk reads as one handed it, through JSON too; none is drawn only for a chunk no codec encodes."""
282302
drawn = pipelines(chunk)
@@ -419,6 +439,14 @@ def test_a_codec_strategy_refusing_an_unknown_chunk_is_a_value_error() -> None:
419439
fields(GZIP_CODEC, strategies=DEFAULT_STRATEGIES | {GZIP_CODEC: refusing})
420440

421441

442+
def test_fill_values_of_a_field_no_definition_reads_is_a_lookup_error() -> None:
443+
"""A struct handed a field type that no definition in the strategies reads has no fill values to draw for it."""
444+
without_float32 = {d: s for d, s in DEFAULT_STRATEGIES.items() if d is not FLOAT32_DATA_TYPE}
445+
configuration: Any = {"fields": ({"name": "a", "data_type": "float32"},)}
446+
with pytest.raises(LookupError, match="no definition in `strategies` reads"):
447+
fill_values(STRUCT_DATA_TYPE, configuration, strategies=without_float32)
448+
449+
422450
def test_a_refused_nested_draw_is_a_value_error() -> None:
423451
"""A field a configuration holds is read as it is drawn, so a strategy that draws a refused one is caught there."""
424452
refused = DataTypeStrategy(
@@ -429,7 +457,7 @@ def test_a_refused_nested_draw_is_a_value_error() -> None:
429457
CAST_VALUE_CODEC: DEFAULT_STRATEGIES[CAST_VALUE_CODEC],
430458
NUMPY_DATETIME64_DATA_TYPE: refused,
431459
}
432-
with pytest.raises(ValueError, match="which is refused"):
460+
with pytest.raises(ValueError, match="read in the strategies. scope, is refused"):
433461
find(fields(CAST_VALUE_CODEC, strategies=strategies), lambda _field: True, settings=_ONCE)
434462

435463

@@ -560,16 +588,21 @@ def test_pipelines_reach_every_codec(definition: CodecDefinition[Any]) -> None:
560588
)
561589

562590

563-
@pytest.mark.parametrize("member", ["codecs", "index_codecs"])
564591
@pytest.mark.parametrize(
565-
"definition", [d for d in _CODECS if d is not SHARDING_INDEXED_CODEC], ids=_id
592+
("definition", "member"),
593+
[
594+
(d, member)
595+
for d in _CODECS
596+
if d is not SHARDING_INDEXED_CODEC
597+
for member in ("codecs", "index_codecs")
598+
if member == "codecs" or d.size == "static"
599+
],
600+
ids=_id,
566601
)
567602
def test_shards_hold_every_codec_their_pipeline_takes(
568603
definition: CodecDefinition[Any], member: str
569604
) -> None:
570605
"""A shard's pipelines reach each codec they take: its index codecs, only those of static size."""
571-
if member == "index_codecs" and definition.size != "static":
572-
return
573606
find(
574607
_shards(),
575608
lambda shard: (definition.name, (member,)) in _codecs_in((shard,)),
@@ -612,8 +645,18 @@ def _read_data_type(field: object) -> Any: # noqa: ANN401
612645
return resolved
613646

614647

615-
_REAL_NUMBERS = [d for d in _DATA_TYPES if d.name not in NOT_NUMBERS | COMPLEX]
616-
"""The data types `cast_value` casts to: those that model real numbers."""
648+
_REAL_NUMBERS = [
649+
d
650+
for d in _DATA_TYPES
651+
if d.name
652+
in {
653+
*(f"{sign}int{bits}" for sign in ("", "u") for bits in (8, 16, 32, 64)),
654+
*(f"float{bits}" for bits in (16, 32, 64)),
655+
"numpy.datetime64",
656+
"numpy.timedelta64",
657+
}
658+
]
659+
"""The data types `cast_value` casts to: integers and floats, and the numpy times, which are signed 64-bit integers."""
617660

618661

619662
@pytest.mark.parametrize("definition", _REAL_NUMBERS, ids=_id)

0 commit comments

Comments
 (0)