Skip to content

Commit e48c105

Browse files
d-v-bd-v-b
committed
fix(zarr-metadata)!: guards narrow only when they say yes
`TypeIs[T]` promises True exactly for the values that are `T`, and type checkers narrow `T` away when it returns False. Every `is_*` guard is stricter than its type: `is_json(math.nan)` is False for a `float`, and the document guards are False for well-typed documents that break a structural rule. A consumer narrowing on the False branch type-checked and then failed at run time. The guards are now `TypeGuard`s, which narrow only on True. BREAKING CHANGE: code that relied on a False result narrowing the type now sees the type it passed in. Assisted-by: ClaudeCode:claude-opus-5-5
1 parent b3045a2 commit e48c105

3 files changed

Lines changed: 40 additions & 11 deletions

File tree

‎packages/zarr-metadata/changes/4429.misc.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
**Breaking:** for type checkers, every `is_*` guard in `zarr_metadata.model`
2+
is declared `TypeGuard` rather than `TypeIs`. Each returns False for some
3+
values of its type: `is_json(math.nan)` is False though a NaN is a
4+
`float`, and a document can be well typed and structurally invalid. A
5+
`TypeIs` told type checkers to narrow such a value away when the guard
6+
said no, so code that type-checked could fail at run time. True narrows as
7+
before; False no longer narrows.

‎packages/zarr-metadata/src/zarr_metadata/model/_validation.py‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
literals like `zarr_format`), not domain validity. Each concept gets a
55
`validate_*` function returning every problem found, an `is_*` type guard,
66
and a `parse_*` function that narrows or raises `MetadataValidationError`.
7+
The guards are `TypeGuard`s, not `TypeIs`: True narrows a value to its
8+
document type, and False says nothing about its type, since a value can be
9+
well typed and still not a valid document.
710
811
Every `ValidationProblem` carries a machine-readable `kind` alongside its
912
human-readable `message`, so consumers can dispatch on the failure mode
@@ -17,9 +20,7 @@
1720
import math
1821
from collections.abc import Mapping, Sequence
1922
from dataclasses import dataclass
20-
from typing import Final, Literal, TypeVar, cast
21-
22-
from typing_extensions import TypeIs
23+
from typing import Final, Literal, TypeGuard, TypeVar, cast
2324

2425
from zarr_metadata._common import JSONValue
2526
from zarr_metadata.v2.array import ZarrV2ArrayMetadataJSON
@@ -160,7 +161,7 @@ def _refine(value: object, loc: tuple[str | int, ...], *, finite: bool) -> _Refi
160161
)
161162

162163

163-
def _is_canonical_json(value: object, *, finite: bool = True) -> TypeIs[JSONValue]:
164+
def _is_canonical_json(value: object, *, finite: bool = True) -> TypeGuard[JSONValue]:
164165
"""Whether `value` already uses the concrete containers in `JSONValue`.
165166
166167
A non-finite number counts only when `finite` is false, as a document's
@@ -182,7 +183,7 @@ def _is_canonical_json(value: object, *, finite: bool = True) -> TypeIs[JSONValu
182183
return False
183184

184185

185-
def is_json(value: object) -> TypeIs[JSONValue]:
186+
def is_json(value: object) -> TypeGuard[JSONValue]:
186187
"""Whether `value` is a canonical JSON structure (recursively)."""
187188
return _is_canonical_json(value)
188189

@@ -362,7 +363,7 @@ def validate_metadata_field_v3(
362363
return tuple(problems)
363364

364365

365-
def is_metadata_field_v3(value: object) -> TypeIs[ZarrV3MetadataFieldJSON]:
366+
def is_metadata_field_v3(value: object) -> TypeGuard[ZarrV3MetadataFieldJSON]:
366367
"""Whether `value` is a v3 metadata field: a bare name or a named config."""
367368
if isinstance(value, str):
368369
return True
@@ -621,7 +622,7 @@ def validate_array_metadata_v3(value: object) -> tuple[ValidationProblem, ...]:
621622
return tuple(problems)
622623

623624

624-
def is_array_metadata_v3(value: object) -> TypeIs[ZarrV3ArrayMetadataJSON]:
625+
def is_array_metadata_v3(value: object) -> TypeGuard[ZarrV3ArrayMetadataJSON]:
625626
"""Whether `value` is a structurally-valid v3 array metadata document."""
626627
return (
627628
_is_canonical_json(value, finite=False)
@@ -729,7 +730,7 @@ def validate_array_metadata_v2(value: object) -> tuple[ValidationProblem, ...]:
729730
return tuple(problems)
730731

731732

732-
def is_array_metadata_v2(value: object) -> TypeIs[ZarrV2ArrayMetadataJSON]:
733+
def is_array_metadata_v2(value: object) -> TypeGuard[ZarrV2ArrayMetadataJSON]:
733734
"""Whether `value` is a structurally-valid v2 array metadata document."""
734735
return (
735736
_is_canonical_json(value, finite=False)
@@ -842,7 +843,7 @@ def validate_group_metadata_v3(value: object) -> tuple[ValidationProblem, ...]:
842843
return tuple(problems)
843844

844845

845-
def is_group_metadata_v3(value: object) -> TypeIs[ZarrV3GroupMetadataJSON]:
846+
def is_group_metadata_v3(value: object) -> TypeGuard[ZarrV3GroupMetadataJSON]:
846847
"""Whether `value` is a structurally-valid v3 group metadata document."""
847848
return _is_canonical_json(value, finite=False) and not validate_group_metadata_v3(value)
848849

@@ -875,7 +876,7 @@ def validate_group_metadata_v2(value: object) -> tuple[ValidationProblem, ...]:
875876
return tuple(problems)
876877

877878

878-
def is_group_metadata_v2(value: object) -> TypeIs[ZarrV2GroupMetadataJSON]:
879+
def is_group_metadata_v2(value: object) -> TypeGuard[ZarrV2GroupMetadataJSON]:
879880
"""Whether `value` is a structurally-valid v2 group metadata document."""
880881
return _is_canonical_json(value, finite=False) and not validate_group_metadata_v2(value)
881882

‎packages/zarr-metadata/tests/model/test_array.py‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import json
66
from collections import UserDict
77
from collections.abc import Callable
8-
from typing import TYPE_CHECKING, get_args
8+
from typing import TYPE_CHECKING, TypeGuard, get_args, get_origin, get_type_hints
99

1010
import pytest
1111
from typing_extensions import Unpack
@@ -26,6 +26,8 @@
2626
ZarrV3NamedConfig,
2727
is_array_metadata_v2,
2828
is_array_metadata_v3,
29+
is_group_metadata_v2,
30+
is_group_metadata_v3,
2931
is_json,
3032
is_metadata_field_v3,
3133
parse_array_metadata_v2,
@@ -951,6 +953,25 @@ def test_parse_json_materializes_abstract_containers() -> None:
951953
json.dumps(parsed, allow_nan=False)
952954

953955

956+
@pytest.mark.parametrize(
957+
"guard",
958+
[
959+
is_json,
960+
is_metadata_field_v3,
961+
is_array_metadata_v3,
962+
is_array_metadata_v2,
963+
is_group_metadata_v3,
964+
is_group_metadata_v2,
965+
],
966+
ids=lambda guard: guard.__name__,
967+
)
968+
def test_a_guard_narrows_only_when_it_says_yes(guard: Callable[[object], bool]) -> None:
969+
"""Each guard is False for some values of its type -- `is_json(math.nan)` is,
970+
and a NaN is a `float` -- so it is a `TypeGuard`: a `TypeIs` would tell a
971+
type checker to narrow such a value away when the guard says no."""
972+
assert get_origin(get_type_hints(guard)["return"]) is TypeGuard
973+
974+
954975
def test_json_type_guard_rejects_abstract_sequence() -> None:
955976
"""A guard cannot narrow an abstract sequence that only the parser materializes."""
956977
assert not is_json(range(3))

0 commit comments

Comments
 (0)