From 579f1f8053c5638e0a72a9e274f39b9547cf556e Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 15 Aug 2026 18:10:31 +0200 Subject: [PATCH 1/2] Keep an annotation value of 1 through serialization `OpticalPathAnnotation.value` defaults to True so a bare flag needs no value, and `to_yaml` drops what is still its default. But `1 == True`, so a group numbered from one lost every `1` on the way out: the document reloaded with a boolean where a number had been, the group then held two kinds of value, and the whole inventory was refused. The tunnel recipe does exactly this -- boreholes numbered 1, 2, 3 -- so its doc-code test has been failing on dev since the recipe merged. Identity is what "still its default" means for a field whose type includes both bool and int, so the value goes back into the document unless it is True itself. A flag annotation still writes no value. --- dascore/core/inventory.py | 16 ++++++++++++ tests/test_core/test_inventory.py | 43 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/dascore/core/inventory.py b/dascore/core/inventory.py index ca53d7cda..dfda1e134 100644 --- a/dascore/core/inventory.py +++ b/dascore/core/inventory.py @@ -37,6 +37,7 @@ BeforeValidator, Field, field_validator, + model_serializer, model_validator, ) from typing_extensions import Self @@ -697,6 +698,21 @@ def _reject_empty_string(cls, value): raise ValueError(msg) return value + @model_serializer(mode="wrap") + def _keep_the_value(self, handler, info): + """ + Put back a value which is the default only by ``==``. + + ``exclude_defaults`` compares with ``==``, and ``1 == True``, so a + group numbered from one loses every ``1`` on the way out and reloads + holding a boolean -- which then mixes kinds with the numbers beside + it and is refused. Identity is what "still its default" means here. + """ + out = handler(self) + if "value" not in out and self.value is not True: + out["value"] = self.value + return out + # The coordinates a DistanceMap may be written in, in preference order. DISTANCE_MAP_AXES = ("channel", "instrument_distance") diff --git a/tests/test_core/test_inventory.py b/tests/test_core/test_inventory.py index bdc6378a5..5198899b6 100644 --- a/tests/test_core/test_inventory.py +++ b/tests/test_core/test_inventory.py @@ -2073,6 +2073,49 @@ def test_round_trip_equals(self, name): inventory = SAMPLE_INVENTORIES[name] assert dc.inventory(inventory.to_yaml()) == inventory + def test_an_annotation_value_of_one_survives(self): + """`1 == True`, and the value's default is True, so it was dropped.""" + pytest.importorskip("yaml") + path = inv.OpticalPath( + optical_components=(inv.FiberSegment(optical_length=100.0),), + annotations=( + inv.OpticalPathAnnotation( + start_distance=0.0, end_distance=10.0, group="hole", value=1 + ), + inv.OpticalPathAnnotation( + start_distance=20.0, end_distance=30.0, group="hole", value=2 + ), + ), + ) + array = inv.FiberArray(code="L001", optical_paths=(path,)) + inventory = inv.Inventory( + networks=(inv.Network(code="XX", fiber_arrays=(array,)),) + ) + text = inventory.to_yaml() + assert "value: 1" in text + # Without the value, the group reloads holding a boolean beside a + # number and is refused as mixing two kinds. + assert dc.inventory(text) == inventory + + def test_a_flag_annotation_stays_terse(self): + """A value which really is the default is still left out.""" + pytest.importorskip("yaml") + path = inv.OpticalPath( + optical_components=(inv.FiberSegment(optical_length=100.0),), + annotations=( + inv.OpticalPathAnnotation( + start_distance=0.0, end_distance=10.0, group="noisy" + ), + ), + ) + array = inv.FiberArray(code="L001", optical_paths=(path,)) + inventory = inv.Inventory( + networks=(inv.Network(code="XX", fiber_arrays=(array,)),) + ) + text = inventory.to_yaml() + assert "value:" not in text + assert dc.inventory(text) == inventory + def test_round_trip_through_file(self, tmp_path): """The writer taking a path writes what the text form holds.""" pytest.importorskip("yaml") From 6ea85c672b6d101e012a8f811ef0cf948bb42973 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 15 Aug 2026 19:23:29 +0200 Subject: [PATCH 2/2] Keep the class tag, and the caller's filtering, with the value Declaring a second model serializer took the base's place rather than sitting beside it -- pydantic runs one per class -- so every annotation document lost the object_type it is dispatched by. And putting the value back whenever it was absent overrode a caller who had asked for it to be left out, through exclude or include. The base's serializer is now extended rather than replaced, and the value goes back only where exclude_defaults dropped it: what anyone filtered on purpose stays filtered. --- dascore/core/inventory.py | 39 +++++++++++++++++++++---------- tests/test_core/test_inventory.py | 16 +++++++++++++ 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/dascore/core/inventory.py b/dascore/core/inventory.py index dfda1e134..ebc99797a 100644 --- a/dascore/core/inventory.py +++ b/dascore/core/inventory.py @@ -37,7 +37,6 @@ BeforeValidator, Field, field_validator, - model_serializer, model_validator, ) from typing_extensions import Self @@ -666,6 +665,14 @@ class CouplingCondition(_IntervalModel): ) +def _wanted(field: str, info) -> bool: + """Whether a serialization's own include/exclude asked for a field.""" + if (exclude := getattr(info, "exclude", None)) and field in exclude: + return False + include = getattr(info, "include", None) + return not include or field in include + + class OpticalPathAnnotation(_IntervalModel): """ Key/value annotation attached to an interval of an optical path. @@ -698,19 +705,27 @@ def _reject_empty_string(cls, value): raise ValueError(msg) return value - @model_serializer(mode="wrap") - def _keep_the_value(self, handler, info): + def _write_object_type(self, handler, info): """ - Put back a value which is the default only by ``==``. - - ``exclude_defaults`` compares with ``==``, and ``1 == True``, so a - group numbered from one loses every ``1`` on the way out and reloads - holding a boolean -- which then mixes kinds with the numbers beside - it and is refused. Identity is what "still its default" means here. + Tag the document as every model does, and keep the value with it. + + Overridden rather than added beside: pydantic runs one model + serializer per class, so a second one here would take the base's + place and drop the ``object_type`` every document is dispatched by. + + The value itself needs putting back because ``exclude_defaults`` + compares with ``==`` and ``1 == True``, the default. A group + numbered from one would otherwise lose every ``1`` on the way out + and reload holding a boolean, which then mixes kinds with the + numbers beside it and is refused. Identity is what "still its + default" means for a field admitting both. A caller who asked for + the value to be left out is obeyed: this restores what + exclude_defaults dropped, not what anyone chose to filter. """ - out = handler(self) - if "value" not in out and self.value is not True: - out["value"] = self.value + out = super()._write_object_type(handler, info) + if "value" in out or self.value is True or not _wanted("value", info): + return out + out["value"] = self.value return out diff --git a/tests/test_core/test_inventory.py b/tests/test_core/test_inventory.py index 5198899b6..e2ed18b93 100644 --- a/tests/test_core/test_inventory.py +++ b/tests/test_core/test_inventory.py @@ -2097,6 +2097,22 @@ def test_an_annotation_value_of_one_survives(self): # number and is refused as mixing two kinds. assert dc.inventory(text) == inventory + def test_an_annotation_still_names_its_class(self): + """Restoring the value must not displace the document's tag.""" + annotation = inv.OpticalPathAnnotation( + start_distance=0.0, end_distance=1.0, group="hole", value=2 + ) + dumped = annotation.model_dump(mode="json") + assert dumped["object_type"] == "OpticalPathAnnotation" + + def test_a_deliberately_excluded_value_stays_out(self): + """What a caller filtered is not what exclude_defaults dropped.""" + annotation = inv.OpticalPathAnnotation( + start_distance=0.0, end_distance=1.0, group="hole", value=2 + ) + assert "value" not in annotation.model_dump(mode="json", exclude={"value"}) + assert "value" not in annotation.model_dump(mode="json", include={"group"}) + def test_a_flag_annotation_stays_terse(self): """A value which really is the default is still left out.""" pytest.importorskip("yaml")