diff --git a/packages/zarr-metadata/changes/4429.misc.md b/packages/zarr-metadata/changes/4429.misc.md new file mode 100644 index 0000000000..42c586d45b --- /dev/null +++ b/packages/zarr-metadata/changes/4429.misc.md @@ -0,0 +1,7 @@ +**Breaking:** for type checkers, every `is_*` guard in `zarr_metadata.model` +is declared `TypeGuard` rather than `TypeIs`. Each returns False for some +values of its type: `is_json(math.nan)` is False though a NaN is a +`float`, and a document can be well typed and structurally invalid. A +`TypeIs` told type checkers to narrow such a value away when the guard +said no, so code that type-checked could fail at run time. True narrows as +before; False no longer narrows. diff --git a/packages/zarr-metadata/src/zarr_metadata/model/_validation.py b/packages/zarr-metadata/src/zarr_metadata/model/_validation.py index 1560f14a5c..d231a57d60 100644 --- a/packages/zarr-metadata/src/zarr_metadata/model/_validation.py +++ b/packages/zarr-metadata/src/zarr_metadata/model/_validation.py @@ -4,6 +4,9 @@ literals like `zarr_format`), not domain validity. Each concept gets a `validate_*` function returning every problem found, an `is_*` type guard, and a `parse_*` function that narrows or raises `MetadataValidationError`. +The guards are `TypeGuard`s, not `TypeIs`: True narrows a value to its +document type, and False says nothing about its type, since a value can be +well typed and still not a valid document. Every `ValidationProblem` carries a machine-readable `kind` alongside its human-readable `message`, so consumers can dispatch on the failure mode @@ -17,9 +20,7 @@ import math from collections.abc import Mapping, Sequence from dataclasses import dataclass -from typing import Final, Literal, TypeVar, cast, get_args - -from typing_extensions import TypeIs +from typing import Final, Literal, TypeGuard, TypeVar, cast, get_args from zarr_metadata._common import JSONValue from zarr_metadata.v2.array import ZarrV2ArrayMetadataJSON @@ -200,7 +201,7 @@ def _refine(value: object, loc: tuple[str | int, ...], *, finite: bool) -> _Refi ) -def _is_canonical_json(value: object, *, finite: bool = True) -> TypeIs[JSONValue]: +def _is_canonical_json(value: object, *, finite: bool = True) -> TypeGuard[JSONValue]: """Whether `value` already uses the concrete containers in `JSONValue`. A non-finite number counts only when `finite` is false, as a document's @@ -222,7 +223,7 @@ def _is_canonical_json(value: object, *, finite: bool = True) -> TypeIs[JSONValu return False -def is_json(value: object) -> TypeIs[JSONValue]: +def is_json(value: object) -> TypeGuard[JSONValue]: """Whether `value` is a canonical JSON structure (recursively).""" return _is_canonical_json(value) @@ -402,7 +403,7 @@ def validate_metadata_field_v3( return tuple(problems) -def is_metadata_field_v3(value: object) -> TypeIs[ZarrV3MetadataFieldJSON]: +def is_metadata_field_v3(value: object) -> TypeGuard[ZarrV3MetadataFieldJSON]: """Whether `value` is a v3 metadata field: a bare name or a named config.""" if isinstance(value, str): return True @@ -657,7 +658,7 @@ def validate_array_metadata_v3(value: object) -> tuple[ValidationProblem, ...]: return tuple(problems) -def is_array_metadata_v3(value: object) -> TypeIs[ZarrV3ArrayMetadataJSON]: +def is_array_metadata_v3(value: object) -> TypeGuard[ZarrV3ArrayMetadataJSON]: """Whether `value` is a structurally-valid v3 array metadata document.""" return ( _is_canonical_json(value, finite=False) @@ -765,7 +766,7 @@ def validate_array_metadata_v2(value: object) -> tuple[ValidationProblem, ...]: return tuple(problems) -def is_array_metadata_v2(value: object) -> TypeIs[ZarrV2ArrayMetadataJSON]: +def is_array_metadata_v2(value: object) -> TypeGuard[ZarrV2ArrayMetadataJSON]: """Whether `value` is a structurally-valid v2 array metadata document.""" return ( _is_canonical_json(value, finite=False) @@ -873,7 +874,7 @@ def validate_group_metadata_v3(value: object) -> tuple[ValidationProblem, ...]: return tuple(problems) -def is_group_metadata_v3(value: object) -> TypeIs[ZarrV3GroupMetadataJSON]: +def is_group_metadata_v3(value: object) -> TypeGuard[ZarrV3GroupMetadataJSON]: """Whether `value` is a structurally-valid v3 group metadata document.""" return _is_canonical_json(value, finite=False) and not validate_group_metadata_v3(value) @@ -904,7 +905,7 @@ def validate_group_metadata_v2(value: object) -> tuple[ValidationProblem, ...]: return tuple(problems) -def is_group_metadata_v2(value: object) -> TypeIs[ZarrV2GroupMetadataJSON]: +def is_group_metadata_v2(value: object) -> TypeGuard[ZarrV2GroupMetadataJSON]: """Whether `value` is a structurally-valid v2 group metadata document.""" return _is_canonical_json(value, finite=False) and not validate_group_metadata_v2(value) diff --git a/packages/zarr-metadata/tests/model/test_array.py b/packages/zarr-metadata/tests/model/test_array.py index 5df6b259db..a0b094766b 100644 --- a/packages/zarr-metadata/tests/model/test_array.py +++ b/packages/zarr-metadata/tests/model/test_array.py @@ -6,7 +6,7 @@ import pickle from collections import UserDict from collections.abc import Callable -from typing import TYPE_CHECKING, get_args +from typing import TYPE_CHECKING, TypeGuard, get_args, get_origin, get_type_hints import pytest from typing_extensions import Unpack @@ -27,6 +27,8 @@ ZarrV3NamedConfig, is_array_metadata_v2, is_array_metadata_v3, + is_group_metadata_v2, + is_group_metadata_v3, is_json, is_metadata_field_v3, parse_array_metadata_v2, @@ -966,6 +968,25 @@ def test_parse_json_materializes_abstract_containers() -> None: json.dumps(parsed, allow_nan=False) +@pytest.mark.parametrize( + "guard", + [ + is_json, + is_metadata_field_v3, + is_array_metadata_v3, + is_array_metadata_v2, + is_group_metadata_v3, + is_group_metadata_v2, + ], + ids=lambda guard: guard.__name__, +) +def test_a_guard_narrows_only_when_it_says_yes(guard: Callable[[object], bool]) -> None: + """Each guard is False for some values of its type -- `is_json(math.nan)` is, + and a NaN is a `float` -- so it is a `TypeGuard`: a `TypeIs` would tell a + type checker to narrow such a value away when the guard says no.""" + assert get_origin(get_type_hints(guard)["return"]) is TypeGuard + + def test_json_type_guard_rejects_abstract_sequence() -> None: """A guard cannot narrow an abstract sequence that only the parser materializes.""" assert not is_json(range(3))