Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle X | Y union in GenericModel #4977

Merged
merged 5 commits into from Feb 6, 2023
Merged

Handle X | Y union in GenericModel #4977

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a84227
Fix X | Y union syntax breaks GenericModel (#4146)
thenx Jan 23, 2023
c9602f6
Update changes/4146-thenx.md
thenx Jan 23, 2023
ae61d5a
Improve tests
thenx Jan 23, 2023
bbe9f8b
Recreate newstyle union via typing._UnionGenericAlias
thenx Jan 30, 2023
2060efb
Add basic pep-604 union args order caching detection test
thenx Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/4146-thenx.md
View file
Open in desktop
@@ -0,0 +1 @@
Fix X | Y union syntax breaks GenericModel (#4146)
thenx marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 deletions pydantic/generics.py
View file
Open in desktop
@@ -1,4 +1,7 @@
import functools
import operator
import sys
import types
import typing
from typing import (
TYPE_CHECKING,
Expand Down Expand Up @@ -268,6 +271,10 @@ def replace_types(type_: Any, type_map: Mapping[Any, Any]) -> Any:
# See: https://www.python.org/dev/peps/pep-0585
origin_type = getattr(typing, type_._name)
assert origin_type is not None
# PEP-604 syntax (Ex.: list | str) is represented with a types.UnionType object that does not have __getitem__.
# We also cannot use isinstance() since we have to compare types.
if sys.version_info >= (3, 10) and origin_type is types.UnionType: # noqa: E721
return functools.reduce(operator.or_, resolved_type_args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a slightly weird approach, could we use pydantic.typing.convert_generics?

Copy link
Author

@thenx thenx Jan 23, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a closer look at convert_generics, not sure how it fits, but i guess at least it can be recreated with typing._UnionGenericAlias as it's done in convert_generics.

return origin_type[resolved_type_args]

# We handle pydantic generic models separately as they don't have the same
Expand Down
11 changes: 11 additions & 0 deletions tests/test_generics.py
View file
Open in desktop
Expand Up @@ -853,6 +853,17 @@ class Model(GenericModel, Generic[T]):
# example)
assert replace_types(list[Union[str, list, T]], {T: int}) == list[Union[str, list, int]]

if sys.version_info >= (3, 10):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate test with skipif please.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, please can you add a standalone test for unions on a generic model.

There should be examples elsewhere of testing | style unions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate test with skipif please.

done

also, please can you add a standalone test for unions on a generic model.

There should be examples elsewhere of testing | style unions.

kinda done though i don't have a feeling that that's enough and might add some more later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, let me know when it's done.

assert replace_types(list[T] | None, {T: int}) == list[int] | None
assert replace_types(List[str | list | T], {T: int}) == List[str | list | int]
assert replace_types(list[str | list | T], {T: int}) == list[str | list | int]
assert replace_types(list[str | list | list[T]], {T: int}) == list[str | list | list[int]]
assert replace_types(list[Model[T] | None] | None, {T: T}) == list[Model[T] | None] | None
assert (
replace_types(T | list[T | list[T | list[T | None] | None] | None] | None, {T: int})
== int | list[int | list[int | list[int | None] | None] | None] | None
)


def test_replace_types_with_user_defined_generic_type_field():
"""Test that using user defined generic types as generic model fields are handled correctly."""
Expand Down