Fix some Pydantic V2 deprecations - #2234
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The switch to model_dump() introduces a compatibility regression for inputs that only implement .dict() (e.g., Pydantic v1 / pydantic.v1 models), so a fallback is needed to preserve prior behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses deprecation warnings under Pydantic v2 by replacing usages of .dict() with .model_dump() when converting QCArchive/QCSchema model objects into plain dictionaries.
Changes:
- Updated
Molecule.from_qcschema()to callmodel_dump()when converting non-dict inputs to dicts. - Updated a molecule round-trip test to use
model_dump()when mutating the returned QCSchema object as a dict. - Updated the QCArchive interface example notebook to use
model_dump()in demonstration code.
File summaries
| File | Description |
|---|---|
| openff/toolkit/topology/molecule.py | Updates QCSchema input normalization to use model_dump() to avoid Pydantic v2 deprecation warnings. |
| openff/toolkit/_tests/test_molecule.py | Updates test dict conversion to use model_dump() to avoid deprecation warnings. |
| examples/QCArchive_interface/QCarchive_interface.ipynb | Updates example cells to demonstrate model_dump() instead of deprecated .dict(). |
Review details
Suppressed comments (2)
openff/toolkit/topology/molecule.py:4712
- Switching to
model_dump()removes the Pydantic v2 deprecation, but it also stops accepting objects that only implement.dict()(including Pydantic v1 models orpydantic.v1.BaseModel). This is a behavioral regression from the previous implementation and can causefrom_qcschemato raise even though the input is convertible. Consider falling back to.dict()when.model_dump()is not available, and fix the grammar in the error message while touching this block.
qca_object = qca_object.model_dump()
except AttributeError:
raise AttributeError(
f"The input object (type {type(qca_object)=} "
"passed is not and a dict and could not be converted to a dict."
examples/QCArchive_interface/QCarchive_interface.ipynb:593
- Same as above: calling
entry.model_dump()directly may break this example for QCArchive objects that only support.dict(). Consider a small compatibility fallback.
"molecule_from_dict = Molecule.from_qcschema(entry.model_dump())\n",
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ], | ||
| "source": [ | ||
| "entry.dict()" | ||
| "entry.model_dump()" |
| ) | ||
|
|
||
| mol_dict = off_qcschema.dict() | ||
| mol_dict = off_qcschema.model_dump() |
Holding off on this one briefly until I can be sure that
pydantic =2constraints in new releases are okay. I will update this top-level comment as the source of truthFixes #2233
I couldn't find any other usages such as
.json(). When I run tests locally, no Pydantic-related deprecation warnings are thrown