-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix default .charges
#1066
Fix default .charges
#1066
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
_charges: dict[ | ||
TopologyKey | LibraryChargeTopologyKey, | ||
Quantity, | ||
] = PrivateAttr( | ||
default_factory=dict, | ||
) | ||
# TODO: Charge caching doesn't work when this is defined in the model | ||
# _charges: dict[Any, _ElementaryChargeQuantity] = PrivateAttr(default_factory=dict) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A major source of confusion for me - for whatever reason, having this code in breaks the test which checks that the .charges
property is non-empty. It's not clear to me why, and I don't see the same behavior in my best attempt at stripping it out into a MWE. I don't know if I'm doing something wrong with Pydantic magic, the typing machinery, or I just misunderstand something more basic about how classes and attributes work in Python
from typing import Annotated, Any
import numpy
from openff.toolkit import Quantity
from pydantic import BaseModel, ConfigDict, PrivateAttr, WrapSerializer, computed_field
def quantity_json_serializer(
quantity: Quantity,
nxt,
) -> dict:
"""Serialize a Quantity to a JSON-compatible dictionary."""
magnitude = quantity.m
if isinstance(magnitude, numpy.ndarray):
magnitude = magnitude.tolist()
return {
"val": magnitude,
"unit": str(quantity.units),
}
_DistanceQuantity = Annotated[
Quantity,
WrapSerializer(quantity_json_serializer),
]
class _BaseModel(BaseModel):
"""A custom Pydantic model used by other components."""
model_config = ConfigDict(
validate_assignment=True,
arbitrary_types_allowed=True,
)
def model_dump(self, **kwargs) -> dict[str, Any]:
return super().model_dump(serialize_as_any=True, **kwargs)
def model_dump_json(self, **kwargs) -> str:
return super().model_dump_json(serialize_as_any=True, **kwargs)
class Example(_BaseModel):
greeting: str = "Hello"
_secret: dict[int, _DistanceQuantity] = PrivateAttr(default_factory=dict)
def _build_secret(self):
self._secret = {0: Quantity(
numpy.random.random((3, 3)),
"nanometer",
)}
@computed_field
@property
def secret(self) -> _DistanceQuantity:
self._build_secret()
return self._secret
assert len(Example().secret) > 0 # runs without error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Not sure about the pydantic magic either, but I can vouch for the tests failing when the definitions are included in the model.
Thanks! |
Description
Resolves #1052 ... mostly
Checklist