-
Notifications
You must be signed in to change notification settings - Fork 269
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
Support revalidation of parametrized generics #1489
Conversation
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.
What will happen with the following?
class G(BaseModel, Generic[T]):
a: T | None = None
TypeAdapter(G[int]).validate_python(G(a=1))
TypeAdapter(G[int]).validate_python(G(a="a"))
Is it going to fail because even though the instance check passes for both, the revalidation will make it fail?
What about
TypeAdapter(G[int]).validate_python(G[str]())
?
@Viicos, re your questions above: from pydantic import BaseModel, TypeAdapter
from typing import Generic, TypeVar
T = TypeVar("T")
class G(BaseModel, Generic[T]):
a: T | None = None
print(TypeAdapter(G[int]).validate_python(G(a=1)))
#> a=1
print(TypeAdapter(G[int]).validate_python(G(a="a")))
"""
a
Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='a', input_type=str]
For further information visit https://errors.pydantic.dev/2.10/v/int_parsing
""" and print(TypeAdapter(G[int]).validate_python(G[str]()))
#> a=None I realize the second is a bit confusing, but I think if we add a note to the generics docs on the pydantic side then this can be considered expected. |
CodSpeed Performance ReportMerging #1489 will not alter performanceComparing Summary
|
So, I was going to add something like this: def test_generic_origin() -> None:
T = TypeVar('T')
class Model(Generic[T]):
# this is not required, but it avoids `__pydantic_fields_set__` being included in `__dict__`
__slots__ = '__dict__', '__pydantic_fields_set__', '__pydantic_extra__', '__pydantic_private__'
field_a: T
class IntModel(Model[int]):
...
class InitializableAnyModel(Model[Any]):
def __init__(self, field_a):
self.field_a = field_a
v = SchemaValidator(
core_schema.model_schema(
IntModel,
core_schema.model_fields_schema(
{'field_a': core_schema.model_field(core_schema.int_schema())},
),
generic_origin=Model
)
)
assert v.validate_python({'field_a': 1}).field_a == 1
assert v.validate_python({'field_a': '1'}).field_a == 1
# test that we revalidate internal data for loose subclasses (instance of subclass of generic origin)
# in other words, if we have a type like Model[int] and we pass a Model[Any] instance with field_a of type
# `int`, then validation should pass. Similarly, if we have Model[Any] and we pass an instance of Model[int],
# validation should pass. This is easier to more intuitively test in pydantic, so most of the related testing resides there.
assert v.validate_python(InitializableAnyModel(field_a=1)).field_a == 1
assert v.validate_python(InitializableAnyModel(field_a='1')).field_a == 1 But honestly, this is kind of a faff to test in |
I'm okay just putting tests in pydantic but I think if we ever get burned where a change in pydantic-core gets merged because no tests fail and then the relevant tests of generics fail in pydantic, we should probably revisit this decision. |
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
Getting started on pydantic/pydantic#9414 by adding a
generic_origin
attribute to model like schemas.This makes it such that data validated against a parametrized generic type is revalidated IF said data is an instance of a type that is a subclass of the
generic_origin
associated with the model.So, if you have
Model[Any]
as a type and pass inModelSubclass[int](...)
, this will be revalidated becauseModelSubclass
is a subclass ofModel
. Similarly, if you hadModel[int](...)
, this would be revalidated as well.Pydantic companion PR to this one is #10666