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.

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

python 3.9 generic alias Callable from collections module does not behave correctly in generics #2465

Closed
daviskirk opened this issue Mar 3, 2021 · 5 comments · Fixed by #2519
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@daviskirk
Copy link
Contributor

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.8
            pydantic compiled: False
                 install path: /home/user/Documents/Code/pydantic/pydantic
               python version: 3.9.1 (default, Dec 11 2020, 14:32:07)  [GCC 7.3.0]
                     platform: Linux-5.4.0-66-generic-x86_64-with-glibc2.31
     optional deps. installed: ['devtools', 'dotenv', 'email-validator', 'typing-extensions']
from typing import Any, Callable, ClassVar, Dict, Generic, List, Literal, Optional, Sequence, Tuple, Type, TypeVar, Union

For example, the test_iter_contained_typevars test with replacing typing.Callable with collections.abc.Callable:

from collections.abc import Callable
from typing import Dict, Generic, List, Optional, Union

@skip_36
def test_iter_contained_typevars():
    T = TypeVar('T')
    T2 = TypeVar('T2')

    class Model(GenericModel, Generic[T]):
        a: T

    assert list(iter_contained_typevars(Optional[List[Union[str, Model[T], Callable[[T2, T], str]]]])) == [T, T2, T]

results in

----> 1 assert list(iter_contained_typevars(Optional[List[Union[str, Model[T], Callable[[T2, T], str]]]])) == [T, T2, T]

pydantic-39/lib/python3.9/typing.py in inner(*args, **kwds)
    260             except TypeError:
    261                 pass  # All real errors (not unhashable args) are raised below.
--> 262             return func(*args, **kwds)
    263         return inner
    264 

pydantic-39/lib/python3.9/typing.py in __getitem__(self, parameters)
    337     @_tp_cache
    338     def __getitem__(self, parameters):
--> 339         return self._getitem(self, parameters)
    340 
    341 

pydantic-39/lib/python3.9/typing.py in Union(self, parameters)
    449     msg = "Union[arg, ...]: each arg must be a type."
    450     parameters = tuple(_type_check(p, msg) for p in parameters)
--> 451     parameters = _remove_dups_flatten(parameters)
    452     if len(parameters) == 1:
    453         return parameters[0]

pydantic-39/lib/python3.9/typing.py in _remove_dups_flatten(parameters)
    229             params.append(p)
    230 
--> 231     return tuple(_deduplicate(params))
    232 
    233 

pydantic-39/lib/python3.9/typing.py in _deduplicate(params)
    203 def _deduplicate(params):
    204     # Weed out strict duplicates, preserving the first of each occurrence.
--> 205     all_params = set(params)
    206     if len(all_params) < len(params):
    207         new_params = []

TypeError: unhashable type: 'list'

This came up while reading the release notes for python 3.10 where this is changed back: https://docs.python.org/3.10/whatsnew/3.10.html#collections-abc
So in 3.10 this problem won't be present.

However, since I don't think we're testing any of the "collection" module types particularly well, the assumption is that there might be other problems as well.
We should therefore probably add some tests that check these python >= 3.9 features / generic alias cases in case they are missing.

@daviskirk daviskirk added the bug V1 Bug related to Pydantic V1.X label Mar 3, 2021
@daviskirk
Copy link
Contributor Author

I can probably look at this this weekend... I don't think many people are using generics in 3.9 yet

@AmirHmZz
Copy link

Yes you're right but supporting generic types seems to be rational, According to PEP585 legacy typing.List , typing.Dict , ... are Deprecated since python3.9 and will be removed from python.

@PrettyWood
Copy link
Member

PrettyWood commented Mar 13, 2021

I don't know if it's really out of context but I recently gave a shot at using typingx to use get_args and get_origin with exact same behaviour for all python versions above 3.6 that mimic 3.10 (e.g. the origin is always collections.abc.Callable for a callable)
See here for more details.
The goal was to see if this would help for v2 to target more recent versions.
But I don't know if that would solve the whole problem, maybe just help

@daviskirk
Copy link
Contributor Author

daviskirk commented Mar 13, 2021

@PrettyWood Ah yes, I think this will help with almost all of the problems I was thinking about that might potentially come up.
#2519 only handles only a very special case, which is present right now, and is a problem ONLY with Callable and (I think) ONLY in python 3.9, so that will probably be enough for 1.x and then the rest might be handled by typingx if that's planned for 3.10?

I also commented on the general handling of using typing.XXXX vs. collections.abc.XXXX in tests here #2519 (comment) but I don't have a very clear opinion on that.

@daviskirk daviskirk changed the title python 3.9 generic aliases from collections module do not behave correctly in generics python 3.9 generic alias Callable from collections module does not behave correctly in generics Mar 13, 2021
@antonl
Copy link

antonl commented Mar 17, 2021

@daviskirk I think this issue is actually more general than collections not working. Any generic type-alias that is not in typing will fail.

Here's an example:

from typing import Generic, TypeVar, Mapping
from pydantic.generics import GenericModel, replace_types

KT = TypeVar('KT')
VT = TypeVar('VT')

class ModelWithMappingSubclass(GenericModel, Generic[KT, VT]):
    class GenericMapping(Mapping[KT, VT]):
        pass

    mapping: GenericMapping[KT, VT]

assert replace_values(ModelWithMappingSubclass[KT, VT], {KT: str, VT: str}) == ModelWithMappingSubclass[str, str]

I discovered this when trying to use bidict.frozenbidict as a field. I made a little patch here: https://github.com/antonl/pydantic/tree/patch-generic-mappings .

daviskirk added a commit to daviskirk/pydantic that referenced this issue Mar 20, 2021
daviskirk added a commit to daviskirk/pydantic that referenced this issue Mar 25, 2021
daviskirk added a commit to daviskirk/pydantic that referenced this issue Mar 25, 2021
daviskirk added a commit to daviskirk/pydantic that referenced this issue Apr 11, 2021
samuelcolvin added a commit that referenced this issue May 9, 2021
Work on #2465 (see comments)

Co-authored-by: Samuel Colvin <s@muelcolvin.com>
samuelcolvin added a commit that referenced this issue May 11, 2021
Work on #2465 (see comments)

Co-authored-by: Samuel Colvin <s@muelcolvin.com>
samuelcolvin added a commit that referenced this issue May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V1 Bug related to Pydantic V1.X
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants