fix(zarr-metadata)!: guards narrow only when they say yes

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.

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)
 

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))