diff --git a/packages/essinstruments/tests/loki/spec_digests.json b/packages/essinstruments/tests/loki/spec_digests.json new file mode 100644 index 00000000..438da84c --- /dev/null +++ b/packages/essinstruments/tests/loki/spec_digests.json @@ -0,0 +1,7 @@ +{ + "loki-beam-centre/v1": "8ecc60fbdaca2a91", + "loki-calibrated-detector/v1": "31b35013cb34f95c", + "loki-create-mask/v1": "f5fc5f6545f3b74d", + "loki-iofq/v1": "49a8fc1845f68d66", + "loki-masks/v1": "81f547b6fb3bcaea" +} diff --git a/packages/essinstruments/tests/loki/specs_test.py b/packages/essinstruments/tests/loki/specs_test.py index 34866836..14fcf72a 100644 --- a/packages/essinstruments/tests/loki/specs_test.py +++ b/packages/essinstruments/tests/loki/specs_test.py @@ -2,11 +2,18 @@ # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) import subprocess import sys +from pathlib import Path import pytest +from ess.instruments.loki import specs from ess.instruments.loki.specs import AngleRange, BeamCentre from pydantic import ValidationError +from ess.spec import WorkflowSpec +from ess.spec.testing import check_digests + +DIGESTS = Path(__file__).with_name('spec_digests.json') + def test_specs_load_without_the_workflow_package() -> None: code = ( @@ -18,6 +25,12 @@ def test_specs_load_without_the_workflow_package() -> None: assert result.returncode == 0 +def test_each_spec_has_the_digest_kept_for_its_version() -> None: + check_digests( + [s for s in vars(specs).values() if isinstance(s, WorkflowSpec)], DIGESTS + ) + + @pytest.mark.parametrize('z', [0.0, -1.0]) def test_a_beam_centre_without_a_distance_from_the_sample_is_refused(z: float) -> None: with pytest.raises(ValidationError, match='greater than 0'): diff --git a/packages/essspec/docs/params.md b/packages/essspec/docs/params.md index a456e2f5..63dc73c2 100644 --- a/packages/essspec/docs/params.md +++ b/packages/essspec/docs/params.md @@ -330,7 +330,7 @@ A validator raises `ValueError` for a value it does not take; any other exceptio pydantic tries the members of a union in order, so a validator of a member written inline, such as a `model_validator(mode='before')` of `GivenCentre`, also sees a reference to an output, and refuses it with `ValueError`. The record stores a value written inline in JSON form, and the workflow gets it validated from that form. -So the JSON form of a group is the object of its fields: a field has no alias and no `exclude`, and a group has no serializers and no computed fields. +So the JSON form of a group is the object of its fields: a field has no alias, no `exclude`, and no serializer in its type, such as `Annotated[float, PlainSerializer(...)]`, and a group has no serializers and no computed fields. A group may have field validators and model validators of any mode. A value written inline must round-trip through JSON: a validator that changes an already valid value, such as converting millimetres to metres, is refused. @@ -338,6 +338,54 @@ Values that a check compares, such as the start and stop of a range, are fields A check across parameters, such as a Q range the wavelength band can reach, belongs to the workflow and fails when the request runs. Since each parameter is validated alone, and a stage and a driver never get a whole params instance, the params model has no decorators, neither validators of any mode nor serializers nor computed fields, and no `__init__` of its own. +## The digest of a spec + +`spec.digest` is a short hex string computed from the name, type, and requiredness of each field of the params and outputs models, and of each group inside them, at any depth. +It tells a record made with other models under the same name and version from one of the bound spec ([ADR 0009](../../../docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md)). + +| Changes the digest | Does not | +|---|---| +| a field renamed, added, or removed | a title, a description, a docstring | +| a field's type, such as `float` to `int`, `X` to `X \| None`, `X` to `Referenced[X]` | a default | +| a field made required or optional | a constraint, such as `Field(gt=0)`, or a validator | +| the values of a `Literal` or an enum | the order of fields, or of the members of a union | +| the metadata of a `File(...)` marker | the name or module of a group's class | +| the `extra` setting of a group | the names of an enum's members, or an enum replaced by a `Literal` of its values | +| the root type of a `RootModel` | | +| the module or name of a class that is not a group | | + +A type is described so: + +```python +class Centre(BaseModel): # a group: by its fields, and by `extra` unless default + x: float # a builtin, a time, Path, a scipp type: by its name + unit: Literal['mm', 'm'] # a Literal or an enum: by its values, {'values': ['m', 'mm']} + +class Masks(RootModel[dict[str, sc.Variable]]): ... # by its root type, dict[str, Variable] + +class Shapes(RootModel[list[Polygon]]): ... # any other class, such as Polygon: by module and name, 'mypackage.shapes.Polygon' +``` + +Changing a file field's metadata, including the role of a raw file, changes the digest. +`Referenced[T]` is described by the fields of `T`, since a reference is checked by class when it is used: `T` replaced by another class with the same fields keeps the digest, and a reference made with the old class is refused when it is checked. +Requiredness counts, although a record stores its params with the defaults filled in, since a parameter that becomes required breaks callers. +Serialization settings of a group's `model_config` other than `extra`, such as `ser_json_inf_nan`, are not covered. +A type that is not resolved, such as a forward reference before `model_rebuild()`, has no digest: `spec.digest` raises, naming the field. +The digest does not see a change of what the workflow computes, so the version stays the author's statement of any change a consumer can observe. + +A package keeps the digests of its specs in a file and checks them in its tests, so that a model changed without a new version fails there: + +```python +from ess.spec.testing import check_digests, write_digests + +check_digests([IOFQ, BEAM_CENTRE], Path(__file__).with_name('spec_digests.json')) +write_digests([IOFQ, BEAM_CENTRE], path) # once, to make the file: {"sans-iofq/v1": "9f2c...", ...} +``` + +The check names each spec whose digest differs from the file's, each spec the file lacks, with the entry the file needs, and each entry of the file that no spec given has. +The file holds the given specs and nothing else, so a test that finds no specs fails. +A changed interface gets a new version; the file is updated under the same version only for a change that is meant to keep it, knowing that records of the old models are then of another spec. + ## What `WorkflowSpec` refuses Making a spec raises `pydantic.ValidationError`, which lists each problem with the model and field it concerns. @@ -394,4 +442,5 @@ Params model and groups ([When values are validated](#when-values-are-validated) - A field has no alias (`alias`, `validation_alias`, `serialization_alias`) and no `exclude` or `exclude_if`. - A group has no serializers and no computed fields. +- A field's type has no serializer written with `Annotated`, such as `PlainSerializer` or `WrapSerializer`. - The params model has no decorators (validators of any mode, serializers, computed fields) and no `__init__` of its own. diff --git a/packages/essspec/src/ess/spec/_workflow_spec.py b/packages/essspec/src/ess/spec/_workflow_spec.py index 8483d4a2..ce99f07d 100644 --- a/packages/essspec/src/ess/spec/_workflow_spec.py +++ b/packages/essspec/src/ess/spec/_workflow_spec.py @@ -48,12 +48,17 @@ from __future__ import annotations +import hashlib +import json from collections.abc import Callable, Sequence +from enum import Enum from pathlib import Path, PurePath from types import NoneType, UnionType from typing import ( Annotated, Any, + ForwardRef, + Literal, Self, TypeAliasType, Union, @@ -66,6 +71,9 @@ BaseModel, ConfigDict, Field, + PlainSerializer, + RootModel, + WrapSerializer, model_validator, ) from pydantic.fields import FieldInfo @@ -159,7 +167,8 @@ class WorkflowSpec[P: BaseModel, O: BaseModel](_SpecFields, frozen=True): version: Version of the workflow interface, at least 1. Increase it on any change that a consumer can observe: the params, the outputs, or what - the workflow computes. + the workflow computes. :attr:`digest` detects a change of the models + under one version. title: Display title of the workflow, not empty. description: @@ -218,6 +227,64 @@ def _check_models(self) -> Self: raise ValueError(f'{self.name}: {"; ".join(problems)}') return self + @property + def digest(self) -> str: + """ + A short hex string that changes when the stored form of the params or + the outputs changes, so that a record of the spec can tell whether the + spec it was made with had the models of this one. + + It covers the name, type, and requiredness of each field of the params + and outputs models, and of each group inside them, at any depth. + Requiredness counts although a record stores its params with the + defaults filled in, since a parameter that becomes required breaks + callers. A type is described so: + + - a group, a pydantic model that is not a ``RootModel``, by its fields + and by its ``extra`` setting unless that is the default, not by its + class name or module, so that a group may be renamed or moved; + - a ``RootModel`` by the type of its root; + - a ``Literal`` and an enum by their values, which the stored form + holds, so that the two are the same and an enum member may be + renamed; + - ``Referenced[T]`` as a reference to ``T`` and its fields, and a file + field by the metadata of its ``File(...)`` marker, the role of a raw + file included, which decide which files fit; + - a union by its members, a list, a dict, or a tuple by its type + arguments, in order; + - a builtin, such as ``float`` or ``list``, a time, ``Path``, and a + scipp type, such as ``DataArray``, by its name; + - any other class by its module and qualified name, such as + ``mypackage.masks.MaskSet``, so that moving or renaming it changes + the digest. + + The order of fields, and of the members of a union, does not count. + A reference is checked by class when it is used, so ``T`` replaced by + another class of the same fields keeps the digest, and a reference + made with the old class is refused when it is checked. + + Titles, descriptions, defaults, constraints such as ``Field(gt=0)``, + validators, discriminators, and the spec's name and version are not + part of it: they do not change the stored form of a value. Settings + of a group's ``model_config`` that change its JSON form, other than + ``extra``, such as ``ser_json_inf_nan``, are not covered. The digest + does not see a change of what the workflow computes, so the version + stays the author's statement of any change a consumer can observe. + ``ess.spec.testing.check_digests`` checks that a changed digest comes + with a new version. + + Raises + ------ + ValueError + If the type of a field is not resolved, such as a forward + reference before ``model_rebuild()``, or is not made of classes. + """ + interface = { + 'params': _interface(self.params, ()), + 'outputs': _interface(self.outputs, ()), + } + return hashlib.sha256(_canonical(interface).encode()).hexdigest()[:16] + def serialize(self) -> SerializedWorkflowSpec: """ Project to the plain-data form. @@ -344,7 +411,8 @@ def _fields_problems( The problems of each field of ``model``, the params model or a group, inside the groups ``stack``, the params model first. Each field is typed as :func:`_type_problems` checks, has no alias, is not excluded from the - JSON form, and has a default that is a value (see + JSON form, has no serializer in its type (see + :func:`_annotated_serializers`), and has a default that is a value (see :func:`_default_problem`). """ kind = 'a parameter' if len(stack) == 1 else 'a field of a group' @@ -361,6 +429,9 @@ def _fields_problems( ) continue metadata = field_types.field_metadata(field) + if found := _annotated_serializers(field.annotation, metadata): + problems.append(f'{where}: no serializers, {found}') + continue if found := _type_problems(field.annotation, metadata, stack): problems.extend(f'{where}: {problem}' for problem in found) elif field.default_factory_takes_validated_data: @@ -370,6 +441,25 @@ def _fields_problems( return problems +def _annotated_serializers(annotation: Any, metadata: tuple[Any, ...]) -> list[str]: + """ + The serializers written with ``Annotated``, such as ``PlainSerializer``, + in ``metadata`` or in ``annotation``, not inside a class in it: each makes + the JSON form of a value differ from that of its type, as a decorator + serializer of a group does. + """ + found = [type(m).__name__ for m in metadata if isinstance(m, _SERIALIZER_MARKERS)] + if get_origin(annotation) is Annotated: + inner = get_args(annotation)[0] + return [*found, *_annotated_serializers(inner, annotation.__metadata__)] + for arg in get_args(annotation): + found.extend(_annotated_serializers(arg, ())) + return found + + +_SERIALIZER_MARKERS = (PlainSerializer, WrapSerializer) + + def _type_problems( annotation: Any, metadata: tuple[Any, ...], stack: tuple[type[BaseModel], ...] ) -> list[str]: @@ -804,3 +894,93 @@ def _output_type_schema(annotation: Any) -> dict[str, Any]: else: schema = {'type': 'object', 'additionalProperties': _output_type_schema(item)} return {'anyOf': [schema, {'type': 'null'}]} if is_optional else schema + + +def _interface(model: type[BaseModel], stack: tuple[type[BaseModel], ...]) -> Any: + """ + The name, type, and requiredness of each field of ``model``, a group + inside the groups ``stack``, as plain data for :attr:`WorkflowSpec.digest`. + + Raises + ------ + ValueError + If the type of a field cannot be described, naming the field. + """ + if model in stack: + # A group that contains itself, which only an output may hold: how far up. + return {'recursive': len(stack) - stack.index(model)} + described = {} + for name, info in model.model_fields.items(): + try: + tp = _type_interface(field_types.field_type(info), (*stack, model)) + except _Undescribed as error: + raise ValueError( + f'{model.__name__}.{name}: {error}; the digest describes a field ' + 'by its type, so each type in it is a class, resolved' + ) from None + described[name] = {'type': tp, 'required': info.is_required()} + return described + + +class _Undescribed(Exception): + """A type that :func:`_type_interface` cannot describe.""" + + +_SHORT_NAMED = (Path, *field_types.TIMES) +""" +Classes outside ``builtins`` and scipp described by their name alone, which no +other class of a spec takes. +""" + + +def _type_interface(tp: Any, stack: tuple[type[BaseModel], ...]) -> Any: + """The type ``tp`` as plain data, by the rules of :attr:`WorkflowSpec.digest`.""" + origin, args = get_origin(tp), get_args(tp) + if origin is Annotated: + if REFERENCED in tp.__metadata__: + return {'referenced': _type_interface(args[0], stack)} + if (marker := field_types.file_marker(args[0], *tp.__metadata__)) is not None: + return {'file': marker.metadata} + return _type_interface(args[0], stack) + if origin in (Union, UnionType): + members = (_type_interface(arg, stack) for arg in args) + return {'union': sorted(members, key=_canonical)} + # A Literal and an enum of the same values store the same JSON. + if origin is Literal: + values = [v.value if isinstance(v, Enum) else v for v in args] + return {'values': sorted(values, key=_canonical)} + if isinstance(tp, type) and issubclass(tp, Enum): + return {'values': sorted((m.value for m in tp), key=_canonical)} + if field_types.is_group(tp): + group = {'group': _interface(tp, stack)} + if (extra := tp.model_config.get('extra')) not in (None, 'ignore'): + group['extra'] = extra + return group + if isinstance(tp, type) and issubclass(tp, RootModel): + root = tp.model_fields['root'] + return {'root': _type_interface(field_types.field_type(root), stack)} + if origin is not None: + return {_class_name(origin): [_type_interface(a, stack) for a in args]} + if tp is Ellipsis: + return '...' + if isinstance(tp, ForwardRef | str): + raise _Undescribed(f'{tp!r} is not resolved; call model_rebuild() first') + if not isinstance(tp, type): + raise _Undescribed(f'{tp!r} is not a class') + return _class_name(tp) + + +def _class_name(cls: type) -> str: + """ + The name of ``cls`` in a digest: its name alone for a builtin, a time, + ``Path``, or a scipp type, else its module and qualified name. + """ + module = cls.__module__.partition('.')[0] + if cls in _SHORT_NAMED or module in ('builtins', 'scipp'): + return field_types.type_name(cls) + return f'{cls.__module__}.{cls.__qualname__}' + + +def _canonical(data: Any) -> str: + """The JSON form of ``data``, the same for equal data in any process.""" + return json.dumps(data, sort_keys=True, separators=(',', ':')) diff --git a/packages/essspec/src/ess/spec/testing.py b/packages/essspec/src/ess/spec/testing.py index f83e1502..dd8ca0b2 100644 --- a/packages/essspec/src/ess/spec/testing.py +++ b/packages/essspec/src/ess/spec/testing.py @@ -1,8 +1,11 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) """ -Checks of the symmetries that a workflow author promises, for a package's tests. +Checks of what a workflow author promises, for a package's tests. +:func:`check_digests` checks that a spec whose interface changed has a new +version; ``packages/essspec/docs/params.md`` (The digest of a spec) shows how +a package uses it. The other checks are of the symmetries of a binding; ``packages/essspec/docs/bindings.md`` (Testing the symmetries) shows how a package uses them. @@ -17,8 +20,10 @@ from __future__ import annotations import copy +import json import math -from collections.abc import Callable, Iterator, Mapping, Sequence +from collections.abc import Callable, Iterable, Iterator, Mapping, Sequence +from pathlib import Path from typing import Any from pydantic import BaseModel @@ -31,6 +36,72 @@ """A table's rows, as a push gives them.""" +def check_digests(specs: Iterable[WorkflowSpec[Any, Any]], path: Path) -> None: + """ + Raise ``AssertionError`` unless the file at ``path`` keeps the digest of + each spec under the spec's name and version, and nothing else. + + A digest changes when the stored form of the params or the outputs + changes (see :attr:`~ess.spec.WorkflowSpec.digest`). A record made with + the old models is then of another spec than the one bound under the same + name and version, so such a change comes with a new version. The file is + JSON, ``{"loki-iofq/v1": "9f2c..."}``, kept in the package and written by + :func:`write_digests`. The message names each spec whose digest differs, + each spec the file lacks, with the entry that the file would need, and + each entry of the file that no spec has, so that a test that finds no + specs fails. + + Raises + ------ + ValueError + If two specs have the same name and version. + """ + digests = _digests(specs) + kept = json.loads(path.read_text()) if path.exists() else {} + problems = [] + for key, digest in digests.items(): + entry = f'"{key}": "{digest}"' + if key not in kept: + problems.append( + f'{key} is not in {path}: add {entry}, or write the file with ' + 'ess.spec.testing.write_digests' + ) + elif kept[key] != digest: + problems.append( + f'the interface of {key} changed: give the spec a new version, or ' + f'update {path} to {entry} if the change is meant to keep the version' + ) + problems.extend( + f'{key} in {path} is of no spec given: remove it if the spec is gone' + for key in sorted(kept.keys() - digests.keys()) + ) + if problems: + raise AssertionError('; '.join(problems)) + + +def write_digests(specs: Iterable[WorkflowSpec[Any, Any]], path: Path) -> None: + """ + Write the digest of each spec to ``path``, as :func:`check_digests` reads it. + + Raises + ------ + ValueError + If two specs have the same name and version. + """ + path.write_text(json.dumps(dict(sorted(_digests(specs).items())), indent=2) + '\n') + + +def _digests(specs: Iterable[WorkflowSpec[Any, Any]]) -> dict[str, str]: + """The digest of each spec by its key in a file of digests, ``name/vN``.""" + digests: dict[str, str] = {} + for spec in specs: + key = f'{spec.name}/v{spec.version}' + if key in digests: + raise ValueError(f'{key}: two specs of the same name and version') + digests[key] = spec.digest + return digests + + def check_one_row( single: Binding[Any, Any], multi: Binding[Any, Any], diff --git a/packages/essspec/tests/digest_test.py b/packages/essspec/tests/digest_test.py new file mode 100644 index 00000000..bcc47913 --- /dev/null +++ b/packages/essspec/tests/digest_test.py @@ -0,0 +1,285 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2026 Scipp contributors (https://github.com/scipp) +""" +The digest of a spec changes with the name, type, or requiredness of a field +of its params or outputs, at any depth, and with the ``extra`` setting of a +group, and with nothing else. + +Each test places a group ``Centre``, or a variant of it, in a spec at one of +``PLACES``: a parameter, inside a parameter, and the same in the outputs. +""" + +import uuid +from collections.abc import Callable +from enum import Enum +from typing import Annotated, Any, Literal + +import pytest +import scipp as sc +from pydantic import ( + AfterValidator, + BaseModel, + ConfigDict, + Field, + RootModel, + create_model, + field_validator, +) + +from ess.spec import MaskFile, Referenced, RunFile, WorkflowSpec + + +class Centre(BaseModel): + x: float + y: float = 0.0 + + +class Outputs(BaseModel): + n: int + + +ANY_TYPE = ConfigDict(arbitrary_types_allowed=True) + + +def _spec( + params: dict[str, Any] | None = None, outputs: dict[str, Any] | None = None +) -> WorkflowSpec[Any, Any]: + return WorkflowSpec( + name='centre', + version=1, + title='Centre', + description='Finds the centre.', + params=create_model('Params', **(params or {})), + outputs=create_model('Outputs', __config__=ANY_TYPE, **outputs) + if outputs + else Outputs, + ) + + +def _outer(group: type[BaseModel]) -> type[BaseModel]: + return create_model('Outer', centre=(group, ...)) + + +PLACES: dict[str, Callable[[type[BaseModel]], WorkflowSpec[Any, Any]]] = { + 'parameter': lambda g: _spec(params={'centre': (g, ...)}), + 'group in a parameter': lambda g: _spec(params={'outer': (_outer(g), ...)}), + 'list parameter': lambda g: _spec(params={'centres': (list[g], ...)}), # type: ignore[valid-type] + 'reference': lambda g: _spec(params={'centre': (Referenced[g], ...)}), + 'output': lambda g: _spec(outputs={'centre': (g, ...)}), + 'group in an output': lambda g: _spec(outputs={'outer': (_outer(g), ...)}), + 'dict output': lambda g: _spec(outputs={'centres': (dict[str, g] | None, ...)}), # type: ignore[valid-type] +} + + +class Described(BaseModel): + """Another class, with titles, descriptions, defaults, a constraint, a + validator, and the fields in another order.""" + + y: float = Field(default=1.0, title='Y', description='Vertical, in mm.') + x: Annotated[float, AfterValidator(abs)] = Field(gt=0, title='X') + + @field_validator('y') + @classmethod + def _finite(cls, value: float) -> float: + return value + + +class Renamed(BaseModel): + x_mm: float + y: float = 0.0 + + +class Retyped(BaseModel): + x: int + y: float = 0.0 + + +class XOptional(BaseModel): + x: float = 0.0 + y: float = 0.0 + + +class YRequired(BaseModel): + x: float + y: float + + +class Added(BaseModel): + x: float + y: float = 0.0 + z: float = 1.0 + + +class Removed(BaseModel): + x: float + + +@pytest.mark.parametrize('place', PLACES) +def test_titles_descriptions_defaults_and_order_do_not_change_the_digest( + place: str, +) -> None: + assert PLACES[place](Described).digest == PLACES[place](Centre).digest + + +@pytest.mark.parametrize('place', PLACES) +@pytest.mark.parametrize( + 'variant', [Renamed, Retyped, XOptional, YRequired, Added, Removed] +) +def test_a_changed_field_changes_the_digest( + place: str, variant: type[BaseModel] +) -> None: + assert PLACES[place](variant).digest != PLACES[place](Centre).digest + + +class Unit(Enum): + MM = 'mm' + M = 'm' + + +class Metre(Enum): + M = 'm' + + +class UnitRenamed(Enum): + MILLIMETRE = 'mm' + M = 'm' + + +class Given(BaseModel): + kind: Literal['given'] = 'given' + x: float = 0.0 + + +class Find(BaseModel): + kind: Literal['find'] = 'find' + + +@pytest.mark.parametrize( + ('a', 'b'), + [ + (float, float | None), + (Literal['a', 'b'], Literal['a']), + (Unit, Metre), + (Centre, Referenced[Centre]), + (RunFile, MaskFile), + (list[Centre], Centre), + ], +) +def test_a_changed_type_changes_the_digest(a: Any, b: Any) -> None: + assert _spec(params={'p': (a, ...)}).digest != _spec(params={'p': (b, ...)}).digest + + +@pytest.mark.parametrize( + ('a', 'b'), + [ + (Unit, UnitRenamed), + (Unit, Literal['m', 'mm']), + (Literal['mm', 'm'], Literal['m', 'mm']), + ], +) +def test_an_enum_or_literal_of_the_same_values_keeps_the_digest(a: Any, b: Any) -> None: + assert _spec(params={'p': (a, ...)}).digest == _spec(params={'p': (b, ...)}).digest + + +class Forbidding(Centre): + model_config = ConfigDict(extra='forbid') + + +def test_the_extra_setting_of_a_group_changes_the_digest() -> None: + assert PLACES['parameter'](Forbidding).digest != PLACES['parameter'](Centre).digest + + +class IntMasks(RootModel[dict[str, int]]): + pass + + +class StrMasks(RootModel[dict[str, str]]): + pass + + +def test_a_root_model_is_described_by_its_root_type() -> None: + assert ( + _spec(outputs={'masks': (IntMasks, ...)}).digest + != _spec(outputs={'masks': (StrMasks, ...)}).digest + ) + + +def test_a_class_that_is_not_a_group_is_described_by_its_module() -> None: + here = type('Mask', (), {'__module__': 'here'}) + there = type('Mask', (), {'__module__': 'there'}) + assert ( + _spec(outputs={'mask': (here, ...)}).digest + != _spec(outputs={'mask': (there, ...)}).digest + ) + + +def test_an_unresolved_type_has_no_digest() -> None: + class Late(BaseModel): + later: 'Later' # type: ignore[name-defined] # noqa: F821 + + spec = _spec(outputs={'late': (Late, ...)}) + with pytest.raises(ValueError, match=r"^Late\.later: ForwardRef\('Later'\)"): + _ = spec.digest + + +def test_the_order_of_the_members_of_a_union_does_not_change_the_digest() -> None: + a = Annotated[Given | Find, Field(discriminator='kind')] + b = Annotated[Find | Given, Field(discriminator='kind')] + assert _spec(params={'p': (a, ...)}).digest == _spec(params={'p': (b, ...)}).digest + + +def test_a_field_moved_from_the_params_to_the_outputs_changes_the_digest() -> None: + assert ( + _spec(params={'centre': (Centre, ...)}).digest + != _spec(outputs={'centre': (Centre, ...), 'n': (int, ...)}).digest + ) + + +class Node(BaseModel): + value: float + children: list['Node'] + + +def test_an_output_group_that_contains_itself_has_a_digest() -> None: + assert _spec(outputs={'tree': (Node, ...)}).digest != _spec().digest + + +# Literals, so that a change of the digest's rules, or of what pydantic or Python +# report of a type, fails here rather than in a workspace's log. One per kind of +# description. +STABLE = { + 'scalar': (lambda: _spec(params={'p': (float, ...)}), 'dae9f48e5eab709f'), + 'group': (lambda: PLACES['parameter'](Centre), '9c81351d3a6df500'), + 'nested group': ( + lambda: PLACES['group in a parameter'](Centre), + '5e679ffa780f085e', + ), + 'union with None': ( + lambda: _spec(params={'p': (float | None, None)}), + 'a0fe99b0f1b0f016', + ), + 'Literal': ( + lambda: _spec(params={'p': (Literal['a', 'b'], ...)}), + 'def1a06088db55d0', + ), + 'enum': (lambda: _spec(params={'p': (Unit, ...)}), '32e94d99d33720a4'), + 'file field': (lambda: _spec(params={'p': (MaskFile, ...)}), 'bf81c990bc493a95'), + 'Referenced[T]': (lambda: PLACES['reference'](Centre), '8fd19f5a95e8a310'), + 'list': (lambda: PLACES['list parameter'](Centre), '1d5ed60276784d20'), + 'dict': (lambda: _spec(outputs={'o': (dict[str, float], ...)}), '016544ebacea2bae'), + 'scipp type': ( + lambda: _spec(outputs={'o': (sc.DataArray, ...)}), + '8c37efc2323ffa18', + ), + 'RootModel': (lambda: _spec(outputs={'o': (IntMasks, ...)}), '1cc5f4c89f907304'), + 'class by module': ( + lambda: _spec(outputs={'o': (uuid.UUID, ...)}), + 'b73ba80b6447254a', + ), +} + + +@pytest.mark.parametrize('kind', STABLE) +def test_the_digest_is_the_same_in_every_process(kind: str) -> None: + make, expected = STABLE[kind] + assert make().digest == expected diff --git a/packages/essspec/tests/testing_test.py b/packages/essspec/tests/testing_test.py index 91574d81..f86354fd 100644 --- a/packages/essspec/tests/testing_test.py +++ b/packages/essspec/tests/testing_test.py @@ -2,15 +2,18 @@ # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) """ Each check passes for a binding or driver that keeps its symmetry and fails -for one with a bug that breaks it. +for one with a bug that breaks it. The check of digests passes for the specs +whose digests a file keeps, and fails for a spec changed under its version. The toy spec ``SUB`` takes a sample and a can run and gives ``diff = factor * (sample.counts - can.counts)``; ``SUB_MULTI`` sums the counts of the rows of each table. """ +import re from collections.abc import Callable, Collection, Mapping from dataclasses import dataclass +from pathlib import Path from typing import Any import numpy as np @@ -25,8 +28,10 @@ assert_close, check_arrival_and_order, check_caching, + check_digests, check_one_row, one_row, + write_digests, ) @@ -387,3 +392,76 @@ def masked(data: Measured, scale: Scale) -> DataOutputs: check_caching( Calls(DATA, masked), {'data': Measured(array=data)}, [{'scale': Scale()}] ) + + +class ScaleInPercent(BaseModel): + percent: float = 100.0 + + +def test_digests_pass_for_the_specs_they_were_written_for(tmp_path: Path) -> None: + write_digests([SUB, SUB_MULTI], tmp_path / 'digests.json') + check_digests([SUB, SUB_MULTI], tmp_path / 'digests.json') + + +def test_digests_fail_for_a_spec_whose_interface_changed_under_its_version( + tmp_path: Path, +) -> None: + path = tmp_path / 'digests.json' + write_digests([SUB], path) + + class Changed(SubParams): + scale: ScaleInPercent = Field(default_factory=ScaleInPercent) # type: ignore[assignment] + + changed = _spec('sub', params=Changed, outputs=SubOutputs) + message = ( + 'the interface of sub/v1 changed: give the spec a new version, or update ' + f'{path} to "sub/v1": "{changed.digest}" if the change is meant to keep ' + 'the version' + ) + with pytest.raises(AssertionError, match=f'^{re.escape(message)}$'): + check_digests([changed], path) + + +def test_digests_fail_for_a_spec_the_file_lacks_and_say_how_to_add_it( + tmp_path: Path, +) -> None: + path = tmp_path / 'digests.json' + write_digests([SUB], path) + with pytest.raises( + AssertionError, + match=f'sub-multi/v1 is not in .*: add "sub-multi/v1": "{SUB_MULTI.digest}", ' + 'or write the file with ess.spec.testing.write_digests', + ): + check_digests([SUB, SUB_MULTI], path) + + +def test_digests_fail_for_an_entry_of_no_spec_and_say_to_remove_it( + tmp_path: Path, +) -> None: + path = tmp_path / 'digests.json' + write_digests([SUB, SUB_MULTI], path) + message = ( + f'sub-multi/v1 in {path} is of no spec given: remove it if the spec is gone' + ) + with pytest.raises(AssertionError, match=f'^{re.escape(message)}$'): + check_digests([SUB], path) + + +def test_digests_fail_when_no_spec_is_given(tmp_path: Path) -> None: + path = tmp_path / 'digests.json' + write_digests([SUB], path) + with pytest.raises(AssertionError, match=r'sub/v1 .* is of no spec given'): + check_digests([], path) + + +@pytest.mark.parametrize('function', [check_digests, write_digests]) +def test_digests_refuse_two_specs_of_the_same_name_and_version( + tmp_path: Path, function: Callable[..., None] +) -> None: + other = _spec( + 'sub', params=SubMultiParams, outputs=SubOutputs, tables=('sample', 'can') + ) + with pytest.raises( + ValueError, match=r'^sub/v1: two specs of the same name and version$' + ): + function([SUB, other], tmp_path / 'digests.json') diff --git a/packages/essspec/tests/workflow_spec_test.py b/packages/essspec/tests/workflow_spec_test.py index c2d76dbf..e2f6d442 100644 --- a/packages/essspec/tests/workflow_spec_test.py +++ b/packages/essspec/tests/workflow_spec_test.py @@ -16,7 +16,9 @@ BaseModel, Field, NaiveDatetime, + PlainSerializer, RootModel, + WrapSerializer, computed_field, field_serializer, field_validator, @@ -153,6 +155,10 @@ def _bins(self, bins: int) -> str: return str(bins) +InMilli = Annotated[float, PlainSerializer(lambda v: v * 1000, return_type=float)] +Wrapped = Annotated[int, WrapSerializer(lambda v, handler: handler(v))] + + class SerializedParams(BaseModel): reduction: Reduction = Field(default_factory=Reduction) @@ -1288,6 +1294,22 @@ def test_accepted(self, fields: dict[str, Any]) -> None: r"P.reduction: SerializedReduction: no serializers or computed " r"fields, {'field_serializers': \['_bins'\]}", ), + *( + ({'params': model('P', **fields)}, f"{where}: no serializers, {found}") + for fields, where, found in ( + ({'bins': (InMilli, ...)}, 'P.bins', r"\['PlainSerializer'\]"), + ( + {'bins': (list[InMilli] | None, None)}, + 'P.bins', + r"\['PlainSerializer'\]", + ), + ( + {'run': (model('Run', bins=(Wrapped, ...)), ...)}, + 'P.run: Run.bins', + r"\['WrapSerializer'\]", + ), + ) + ), ( { 'params': model(