Do not try to automatically handle subclasses of mappings #5744
Conversation
Deploying with
|
| Latest commit: |
118055e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28b690e6.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://mapping-subclasses.pydantic-docs2.pages.dev |
|
please review |
|
I would try copying here this test https://github.com/pydantic/pydantic/pull/5598/files#diff-cbcfeba88238560f75e880d530f4cd82ab6bf90247012e8f664281941f8da1f7R3782 from #5598 as well. The test were failing before. |
👍🏻 b0655e0 |
| def test_dict_subclasses_bare(): | ||
| class Model(BaseModel): | ||
| a: MyDict | ||
|
|
||
| assert repr(Model(a=MyDict({'a': 1})).a) == "MyDict({'a': 1})" | ||
| assert repr(Model(a=MyDict({b'x': (1, 2)})).a) == "MyDict({b'x': (1, 2)})" | ||
|
|
||
|
|
||
| def test_dict_subclasses_typed(): | ||
| class Model(BaseModel): | ||
| a: MyDict[str, int] | ||
|
|
||
| assert repr(Model(a=MyDict({'a': 1})).a) == "MyDict({'a': 1})" |
There was a problem hiding this comment.
This is no longer supported, I added tests for it the error
| def test_typing_coercion_defaultdict(): | ||
| class Model(BaseModel): | ||
| x: DefaultDict[int, str] | ||
|
|
||
| d = defaultdict(str) | ||
| d['1'] | ||
| m = Model(x=d) | ||
| assert isinstance(m.x, defaultdict) | ||
| assert repr(m.x) == "defaultdict(<class 'str'>, {1: ''})" | ||
|
|
||
|
|
||
| def test_typing_coercion_counter(): | ||
| class Model(BaseModel): | ||
| x: Counter[str] | ||
|
|
||
| m = Model(x={'a': 10}) | ||
| assert isinstance(m.x, Counter) | ||
| assert repr(m.x) == "Counter({'a': 10})" | ||
|
|
||
|
|
||
| def test_typing_counter_value_validation(): | ||
| class Model(BaseModel): | ||
| x: Counter[str] | ||
|
|
||
| with pytest.raises(ValidationError) as exc_info: | ||
| Model(x={'a': 'a'}) | ||
|
|
||
| # insert_assert(exc_info.value.errors(include_url=False)) | ||
| assert exc_info.value.errors(include_url=False) == [ | ||
| { | ||
| 'type': 'int_parsing', | ||
| 'loc': ('x', 'a'), | ||
| 'msg': 'Input should be a valid integer, unable to parse string as an integer', | ||
| 'input': 'a', | ||
| } | ||
| ] |
| def test_mapping_retains_type_defaultdict(): | ||
| class Model(BaseModel): | ||
| x: Mapping[str, int] | ||
|
|
||
| d = defaultdict(int) | ||
| d['foo'] = '2' | ||
| d['bar'] | ||
|
|
||
| m = Model(x=d) | ||
| assert isinstance(m.x, defaultdict) | ||
| assert m.x['foo'] == 2 | ||
| assert m.x['bar'] == 0 |
There was a problem hiding this comment.
We no longer retain the input type (although we do retain the default value). This also duplicates some of test_replace_types_with_user_defined_generic_type_field
There was a problem hiding this comment.
Probably a good idea to document this in the migration notes, I'll add it to the list (edit: done: https://github.com/orgs/pydantic/projects/3/views/1)
| # `OrderedDict[Any, Any]` | ||
| return _ordered_dict_any_schema() | ||
| # Assume Annotated[..., Field(...)] | ||
| field_info = next((v for v in get_args(values_source_type) if isinstance(v, FieldInfo)), None) |
There was a problem hiding this comment.
This works for now, but I feel like it’s a bit hacky. I think we should try to better integrate this later, eg to give a meaningful error if Field(default_factory=…) is used in places where it isn’t allowed (we currently ignore it)
There was a problem hiding this comment.
It worries me a bit to not actually check that values_source_type is actually Annotated before iterating over the get_args call's results. I'm thinking we could do something like this:
to ensure this and make it safer here in the event that get_args starts handling more things in the future or whatever.
(Might make sense to make that a utility function or similar anyway.)
If you disagree, I don't feel strongly.
| @@ -473,26 +464,10 @@ def _generate_schema(self, obj: Any) -> core_schema.CoreSchema: # noqa: C901 | |||
| elif issubclass(origin, typing.Tuple): # type: ignore[arg-type] | |||
| # TODO: To support generic subclasses of typing.Tuple, we need to better-introspect the args to origin | |||
There was a problem hiding this comment.
Should this comment be removed? Not sure how you feel about generic subclasses of Tuple in the context of this PR
There was a problem hiding this comment.
I think we should do this work for Tuple as well, but I'm not going to do it in this PR, it's already huge 😆
There was a problem hiding this comment.
Maybe worth addressing https://github.com/pydantic/pydantic/pull/5744/files#r1193010982, otherwise lgtm
adriangb
force-pushed
the
mapping-subclasses
branch
from
b0655e0
to
118055e
Compare
OrderedDict[int, Annotated[list, Field(default_factory=...)]]Fixes #3417, #4687
Selected Reviewer: @dmontagu