Fix JSON serialization with pydantic >=2.12#1493
Open
michalkrompiec wants to merge 4 commits intoopenforcefield:mainfrom
Open
Fix JSON serialization with pydantic >=2.12#1493michalkrompiec wants to merge 4 commits intoopenforcefield:mainfrom
michalkrompiec wants to merge 4 commits intoopenforcefield:mainfrom
Conversation
pydantic 2.12 changed serialize_as_any=True to propagate the flag to all nested values (pydantic/pydantic#12348). This causes model_dump_json() to fail with "Unable to serialize unknown type: <class 'pint.Quantity'>" because pydantic now bypasses the Annotated WrapSerializer on _Quantity and tries to serialize pint.Quantity by its runtime type, which has no pydantic schema. The fix removes serialize_as_any=True from model_dump_json(). The Annotated WrapSerializer(quantity_json_serializer) on _Quantity correctly handles JSON serialization without it. All 495 unit tests pass with pydantic 2.13.3.
Author
|
Oops, it turns out I have not run the tests properly locally. Will come back when fixed! |
Without serialize_as_any=True, pydantic serializes collections using the declared Collection base class schema, dropping subclass-specific fields (scale_14, cutoff, periodic_potential, etc.) from _NonbondedCollection and its subclasses. This caused silent data loss on JSON roundtrip, producing wrong electrostatics energies after deserialization. Fix by adding a WrapSerializer to _AnnotatedCollections that calls model_dump_json() on each collection instance directly. Since the call is made on the actual instance (e.g. SMIRNOFFElectrostaticsCollection), pydantic uses the full subclass schema and all fields are preserved.
Author
|
This version passes the py312amber test on my machine. Can you try running the CI again, please? |
- Remove pydantic <2.12 ceiling from docs_env.yaml - Add ci-pydantic-latest.yaml workflow that upgrades pydantic to the latest release after pixi install and runs the full test suite Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Added new pydantic to the environment and added a test explicitly using pydantic>2.12. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1493 +/- ##
==========================================
- Coverage 94.07% 89.73% -4.35%
==========================================
Files 73 73
Lines 6045 6048 +3
==========================================
- Hits 5687 5427 -260
- Misses 358 621 +263 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Author
|
Openeye tests failing due to lack of license keys - is it because the branch is on my account? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pydantic 2.12 changed
serialize_as_any=Trueto propagate the flag to all nested values (pydantic/pydantic#12348). This broke JSON serialization in two ways, requiring two fixes.Fix 1:
pydantic.py— removeserialize_as_any=Truefrommodel_dump_json()With pydantic ≥2.12,
serialize_as_any=Truein JSON mode bypasses theAnnotatedWrapSerializeron_Quantityand tries to serializepint.Quantityby its runtime type, which has no pydantic schema:The
WrapSerializer(quantity_json_serializer)on_Quantityhandles JSON serialization correctly without this flag.serialize_as_any=Trueis intentionally kept inmodel_dump()(Python mode), which is unaffected by the pydantic 2.12 change.Fix 2:
components/potentials.py— addWrapSerializerto_AnnotatedCollectionsWithout
serialize_as_any=True, pydantic serializescollections: dict[str, Collection]using the declaredCollectionbase class schema, silently dropping subclass-specific fields (scale_14,cutoff,periodic_potential, etc. from_NonbondedCollectionand subclasses). This caused wrong electrostatics energies after JSON roundtrip:The fix adds a
WrapSerializerto_AnnotatedCollectionsthat callsmodel_dump_json()on each collection instance directly. Since the call is on the actual instance (e.g.SMIRNOFFElectrostaticsCollection), pydantic uses the full subclass schema and all fields are preserved.Fixes #1346.
Test results (pydantic 2.13.3, after both fixes,
py312amberenvironment)Zero failures. Mypy also passes (
Success: no issues found in 85 source files).