Skip to content

Fix NDVariable round-trip serialization for non-float64 dtypes - #46

Open
jacquelinegarrahan wants to merge 1 commit into
lume-science:mainfrom
jacquelinegarrahan:bugfix/ndvariable-dtype-roundtrip
Open

Fix NDVariable round-trip serialization for non-float64 dtypes#46
jacquelinegarrahan wants to merge 1 commit into
lume-science:mainfrom
jacquelinegarrahan:bugfix/ndvariable-dtype-roundtrip

Conversation

@jacquelinegarrahan

Copy link
Copy Markdown
Contributor

Fix NDVariable round-trip serialization for non-float64 dtypes

Branch: bugfix/ndvariable-dtype-roundtripmain

Bug

lume/variables/ndvariable.py::coerce_default_value (a mode="before"
field_validator for default_value) rebuilt a serialized list into an array
without the model's dtype:

if isinstance(value, (list, tuple)):
    return np.asarray(value)     # dtype inferred as int64 / float64

NumPy infers int64/float64 from a plain list, but validate_default_value
(the mode="after" model validator) requires the array dtype to exactly
match self.dtype. So any NDVariable whose default_value used a different
dtype could not survive model_dump() → reconstruct, even though the
validator's own docstring promises "the model invariants are maintained."

Reproduction:

v = NDVariable(name="a", shape=(2,), dtype=np.dtype("int32"),
               default_value=np.array([1, 2], dtype="int32"))
d = v.model_dump()               # default_value -> [1, 2], dtype -> 'int32'
NDVariable(**{k: val for k, val in d.items() if k != "variable_class"})
# pydantic ValidationError: Expected dtype int32, got int64

Only the float64 default path was covered by tests
(test_round_trip_serialization), so int8/16/32, float32, etc. all failed
silently until exercised.

Fix

Read the already-validated dtype from the pydantic ValidationInfo (safe
because dtype is declared before default_value, so it is validated first)
and pass it to np.asarray, so the reconstructed array matches the declared
dtype. Falls back to NumPy's inference when no dtype is available.

Verification

pytest -q                        # 121 passed

Round-trip now succeeds for int32, float32, int8, float64, and the
no-default case:

int32   round-trip OK -> int32
float32 round-trip OK -> float32
int8    round-trip OK -> int8
float64 round-trip OK -> float64

🤖 Generated with Claude Code

coerce_default_value rebuilt a serialized list with np.asarray(value),
ignoring the model's dtype. NumPy then inferred int64/float64, which
failed the exact-dtype check in validate_default_value on reconstruction.
Any NDVariable with an int8/16/32, float32, etc. default_value therefore
could not survive model_dump() -> reconstruct, despite the validator's
documented round-trip contract (only the float64 path was tested).

Pass the already-validated dtype (dtype is declared before default_value)
into np.asarray so the reconstructed array matches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant