-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Serialize unsubstituted type vars as Any #7606
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1450,11 +1450,17 @@ def _unsubstituted_typevar_schema(self, typevar: typing.TypeVar) -> core_schema. | |
assert isinstance(typevar, typing.TypeVar) | ||
|
||
if typevar.__bound__: | ||
return self.generate_schema(typevar.__bound__) | ||
schema = self.generate_schema(typevar.__bound__) | ||
elif typevar.__constraints__: | ||
return self._union_schema(typing.Union[typevar.__constraints__]) # type: ignore | ||
schema = self._union_schema(typing.Union[typevar.__constraints__]) # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Constraints feel closer to default than to a bound to me, I would personally make them function the old way. I’ll leave it up to you though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay changed |
||
elif hasattr(typevar, '__default__'): | ||
return self.generate_schema(getattr(typevar, '__default__')) | ||
else: | ||
return core_schema.any_schema() | ||
schema['serialization'] = core_schema.wrap_serializer_function_ser_schema( | ||
lambda x, h: h(x), schema=core_schema.any_schema() | ||
) | ||
return schema | ||
|
||
def _computed_field_schema( | ||
self, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2622,3 +2622,64 @@ class Model(Generic[T], BaseModel): | |
m1 = Model[int](x=1) | ||
m2 = Model[int](x=1) | ||
assert len({m1, m2}) == 1 | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'type_var', | ||
[ | ||
TypeVar('ErrorDataT', bound=BaseModel), | ||
TypeVar('ErrorDataT', BaseModel, str), | ||
], | ||
) | ||
def test_serialize_unsubstituted_typevars_bound_or_constraint( | ||
type_var: TypeVar, | ||
) -> None: | ||
class ErrorDetails(BaseModel): | ||
foo: str | ||
|
||
class Error(BaseModel, Generic[type_var]): | ||
message: str | ||
details: Optional[type_var] | ||
|
||
class MyErrorDetails(ErrorDetails): | ||
bar: str | ||
|
||
sample_error = Error( | ||
message='We just had an error', | ||
details=MyErrorDetails(foo='var', bar='baz'), | ||
) | ||
|
||
assert sample_error.model_dump() == { | ||
'message': 'We just had an error', | ||
'details': { | ||
'foo': 'var', | ||
'bar': 'baz', | ||
}, | ||
} | ||
|
||
|
||
def test_serialize_unsubstituted_typevars_default() -> None: | ||
from typing_extensions import TypeVar | ||
|
||
class ErrorDetails(BaseModel): | ||
foo: str | ||
|
||
DataT = TypeVar('DataT', default=ErrorDetails) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice example 👍 |
||
|
||
class Error(BaseModel, Generic[DataT]): | ||
message: str | ||
details: Optional[DataT] | ||
|
||
class MyErrorDetails(ErrorDetails): | ||
bar: str | ||
|
||
sample_error = Error( | ||
message='We just had an error', | ||
details=MyErrorDetails(foo='var', bar='baz'), | ||
) | ||
assert sample_error.model_dump() == { | ||
'message': 'We just had an error', | ||
'details': { | ||
'foo': 'var', | ||
}, | ||
} |
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.
@dmontagu I think this is perhaps the controversial part of this PR. My reasoning was that a bound and default are different semantically and this distinction led me to implement other behavior. This is also an opportunity to preserve the
SerializeAsAny
example that was here before. Let me know what you think.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.
I agree with your interpretation of the semantics. (And in particular, this choice.)