Description
Serializing an inventory drops fields whose value is empty, so that reloading gives back the field's default. That is lossless only when the default is itself the empty value — and for four fields it is not. The most consequential is CoordinateReferenceSystem: a frame deliberately left without an authority code (a local or derived CRS described by WKT) reloads as EPSG:4979 / WGS 84 3D, silently changing what every coordinate in the inventory means.
The pruning happens in _drop_empty (dascore/core/inventory.py), which walks the dumped mapping by key without knowing which model each mapping came from, so it cannot tell "empty and defaults to empty" from "empty and defaults to something else".
Fields with a non-empty string default, and so affected:
| Model |
Field |
Default |
CoordinateReferenceSystem |
authority |
"EPSG" |
CoordinateReferenceSystem |
code |
"4979" |
CoordinateReferenceSystem |
name |
"WGS 84 3D" |
Interrogator |
instrument_type |
"interrogator" |
(object_type on the resource models has the same shape but is a discriminator tag, never blanked by a caller.)
This is long-standing, not a regression — it reproduces identically on dev before the inventory-model-trims branch. _UNPRUNED_KEYS was an exemption list for exactly this hazard but only ever held "value"; that branch removes it because the one entry had become unreachable (an annotation value cannot be the empty string — a validator refuses it), and corrects the docstring to state this limitation instead of claiming losslessness.
Example
import dascore as dc
import dascore.core.inventory as inv
# a local frame described by WKT, deliberately with no authority code
crs = inv.CoordinateReferenceSystem(
authority="", code="", name="", wkt='LOCAL_CS["mine"]',
coordinate_labels=("x", "y"), units=("m", "m"),
)
one = inv.Inventory(coordinate_reference_system=crs)
back = dc.inventory(one.to_yaml()).coordinate_reference_system
print((crs.authority, crs.code, crs.name)) # ('', '', '')
print((back.authority, back.code, back.name)) # ('EPSG', '4979', 'WGS 84 3D')
# same shape, second field
i = inv.Interrogator(resource_id="i1", instrument_type="")
two = inv.Inventory(resources={"i1": i})
print(dc.inventory(two.to_yaml()).resources["i1"].instrument_type) # 'interrogator'
Expected behavior
An inventory written with to_yaml and read back should equal the one written. A field explicitly left blank should come back blank, not as the default it was deliberately not given.
Two ways to get there:
- Prune with the models in hand. Walk the model tree alongside the dump so each key is compared against its own field default. Exact, and removes the need for any exemption list — but
_drop_empty currently takes only the dumped value, so it means threading the model (or dumping and pruning in one pass).
- Derive the exemption list. Recompute the old
_UNPRUNED_KEYS as "every field name whose default is not the empty value", rather than hand-listing it. Cheap, and cannot go stale — but it is keyed on names across all models, so name and code (which are legitimately "" on FiberSegment, Network, and others) would stop being pruned everywhere, making dumps noticeably larger.
(1) is correct; (2) is a few lines. Worth deciding which before the release, since the CRS case silently changes the meaning of stored coordinates rather than failing loudly.
Testing
Whichever fix lands, the check that belongs with it is a round-trip property test over many generated inventories rather than a case per field — the four fields above were found by reading defaults, and reading defaults is exactly what a person does once and a test does every run.
The property: for an arbitrary valid inventory, dc.inventory(inv.to_yaml()) == inv. Same for the JSON form, and for the authoring-directory form (to_yaml → load → compare), since those go through different writers.
What the generator should vary, because each is a way the current pruning can lose something:
- every field explicitly set to its type's empty value (
"", (), {}, 0, 0.0, False) as well as unset — the distinction this bug is about;
- fields whose default is non-empty, set to blank (the four above);
- nested containers at each depth: networks → fiber arrays → optical paths → tracks, with zero, one, and several items;
- the resource pool both inline and by id reference, including one resource referenced from two places;
extra_fields holding values that look prunable ({"a": ""}), since those are kept verbatim and the recursion treats them specially;
- annotation values across all four types the field admits (
str, bool, int, float), including False and 0, which are falsy but not empty.
Hypothesis would fit this well — the models are pydantic, so hypothesis strategies can be built from them, and shrinking would name the offending field directly rather than leaving a large failing inventory to bisect by hand. A plain parametrized generator over the model tree would also do if adding the dependency is unwanted; the value is in generating combinations, not in the framework.
A useful side effect: the same property test would have caught this bug at the time the defaults were introduced, and would catch the next field added with a non-empty default.
Versions
- OS: Linux 6.8 (Ubuntu)
- DASCore Version: 0.1.21.dev159 (
dev, and every earlier version with this pruning)
- Python Version: 3.13.7
Description
Serializing an inventory drops fields whose value is empty, so that reloading gives back the field's default. That is lossless only when the default is itself the empty value — and for four fields it is not. The most consequential is
CoordinateReferenceSystem: a frame deliberately left without an authority code (a local or derived CRS described by WKT) reloads as EPSG:4979 / WGS 84 3D, silently changing what every coordinate in the inventory means.The pruning happens in
_drop_empty(dascore/core/inventory.py), which walks the dumped mapping by key without knowing which model each mapping came from, so it cannot tell "empty and defaults to empty" from "empty and defaults to something else".Fields with a non-empty string default, and so affected:
CoordinateReferenceSystemauthority"EPSG"CoordinateReferenceSystemcode"4979"CoordinateReferenceSystemname"WGS 84 3D"Interrogatorinstrument_type"interrogator"(
object_typeon the resource models has the same shape but is a discriminator tag, never blanked by a caller.)This is long-standing, not a regression — it reproduces identically on
devbefore the inventory-model-trims branch._UNPRUNED_KEYSwas an exemption list for exactly this hazard but only ever held"value"; that branch removes it because the one entry had become unreachable (an annotationvaluecannot be the empty string — a validator refuses it), and corrects the docstring to state this limitation instead of claiming losslessness.Example
Expected behavior
An inventory written with
to_yamland read back should equal the one written. A field explicitly left blank should come back blank, not as the default it was deliberately not given.Two ways to get there:
_drop_emptycurrently takes only the dumped value, so it means threading the model (or dumping and pruning in one pass)._UNPRUNED_KEYSas "every field name whose default is not the empty value", rather than hand-listing it. Cheap, and cannot go stale — but it is keyed on names across all models, sonameandcode(which are legitimately""onFiberSegment,Network, and others) would stop being pruned everywhere, making dumps noticeably larger.(1) is correct; (2) is a few lines. Worth deciding which before the release, since the CRS case silently changes the meaning of stored coordinates rather than failing loudly.
Testing
Whichever fix lands, the check that belongs with it is a round-trip property test over many generated inventories rather than a case per field — the four fields above were found by reading defaults, and reading defaults is exactly what a person does once and a test does every run.
The property: for an arbitrary valid inventory,
dc.inventory(inv.to_yaml()) == inv. Same for the JSON form, and for the authoring-directory form (to_yaml→ load → compare), since those go through different writers.What the generator should vary, because each is a way the current pruning can lose something:
"",(),{},0,0.0,False) as well as unset — the distinction this bug is about;extra_fieldsholding values that look prunable ({"a": ""}), since those are kept verbatim and the recursion treats them specially;str,bool,int,float), includingFalseand0, which are falsy but not empty.Hypothesis would fit this well — the models are pydantic, so
hypothesisstrategies can be built from them, and shrinking would name the offending field directly rather than leaving a large failing inventory to bisect by hand. A plain parametrized generator over the model tree would also do if adding the dependency is unwanted; the value is in generating combinations, not in the framework.A useful side effect: the same property test would have caught this bug at the time the defaults were introduced, and would catch the next field added with a non-empty default.
Versions
dev, and every earlier version with this pruning)