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

list is not equivalent to typing.List under combination of generics and futures #6130

Closed
6 of 15 tasks
mark-todd opened this issue Jun 14, 2023 · 5 comments
Closed
6 of 15 tasks
Labels
bug V1 Bug related to Pydantic V1.X unconfirmed Bug not yet confirmed as valid/applicable

Comments

@mark-todd
Copy link
Contributor

mark-todd commented Jun 14, 2023

Initial Checks

  • I have searched GitHub for a duplicate issue and I'm sure this is something new
  • I have searched Google & StackOverflow for a solution and couldn't find anything
  • I have read and followed the docs and still think this is a bug
  • I am confident that the issue is with pydantic (not my code, or another library in the ecosystem like FastAPI or mypy)

Description

This is a strange one - under the below conditions, exchanging typing.List for the more modern list produces the below error. I can't reproduce in a simpler example than this - it looks to me like a bug in pydantic:

Traceback (most recent call last):
  File "pydantic/validators.py", line 751, in pydantic.validators.find_validators
TypeError: issubclass() arg 1 must be a class

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "{path_to_test}/test.py", line 11, in <module>
    outer = Outer["Inner"]
  File "{path_to_pydantic}/pydantic/generics.py", line 163, in __class_getitem__
    _prepare_model_fields(created_model, fields, instance_type_hints, typevars_map)
  File "{path_to_pydantic}/pydantic/generics.py", line 389, in _prepare_model_fields
    field.prepare()
  File "pydantic/fields.py", line 552, in pydantic.fields.ModelField.prepare
  File "pydantic/fields.py", line 758, in pydantic.fields.ModelField._type_analysis
  File "pydantic/fields.py", line 808, in pydantic.fields.ModelField._create_sub_type
  File "pydantic/fields.py", line 436, in pydantic.fields.ModelField.__init__
  File "pydantic/fields.py", line 557, in pydantic.fields.ModelField.prepare
  File "pydantic/fields.py", line 831, in pydantic.fields.ModelField.populate_validators
  File "pydantic/validators.py", line 760, in find_validators
RuntimeError: error checking inheritance of 'Inner' (type: str)

I've also displayed the minimal example below - thanks!

Example Code

from typing import Generic, List, TypeVar

from pydantic import BaseModel
from pydantic.generics import GenericModel

ChangingType = TypeVar("ChangingType")

class Outer(GenericModel, Generic[ChangingType]):
    fields_: list[ChangingType] # Swapping list -> List on this line fixes the bug

outer = Outer["Inner"]
class Inner(BaseModel):
    new: list[outer]

outer.update_forward_refs()
Inner.update_forward_refs()

out = Inner.parse_obj({"new": [{"fields_": []}]})

print(out)

Python, Pydantic & OS Version

pydantic version: 1.10.9
pydantic compiled: True
install path: /{package location}/.venv/lib64/python3.10/site-packages/pydantic
python version: 3.10.10 (main, Feb  8 2023, 00:00:00) [GCC 12.2.1 20221121 (Red Hat 12.2.1-4)]
platform: Linux-5.10.16.3-microsoft-standard-WSL2-x86_64-with-glibc2.35
optional deps. installed: ['typing-extensions']

Affected Components

@mark-todd mark-todd added bug V1 Bug related to Pydantic V1.X unconfirmed Bug not yet confirmed as valid/applicable labels Jun 14, 2023
@mark-todd
Copy link
Contributor Author

I've found a couple more illustrations - one is a simplified version of the original minimal, and second illustrates the same bug with Dict -> dict

List version

from typing import Generic, List, TypeVar

from pydantic import BaseModel
from pydantic.generics import GenericModel

ChangingType = TypeVar("ChangingType")

class Outer(GenericModel, Generic[ChangingType]):
    test: list[ChangingType]

OuterNew = Outer["Inner"]
class Inner(BaseModel):
    new: int

OuterNew.update_forward_refs()
out = OuterNew.parse_obj({"test": [{"new": 1}]})

print(out)

Dict version

from typing import Dict, Generic, List, TypeVar

from pydantic import BaseModel
from pydantic.generics import GenericModel

ChangingType = TypeVar("ChangingType")

class Outer(GenericModel, Generic[ChangingType]):
    test: dict[str, ChangingType]

OuterNew = Outer["Inner"]
class Inner(BaseModel):
    new: int

OuterNew.update_forward_refs()
out = OuterNew.parse_obj({"test": {"t": {"new": 1}}})

print(out)

@mark-todd
Copy link
Contributor Author

mark-todd commented Jun 14, 2023

https://github.com/pydantic/pydantic/blob/1.10.X-fixes/pydantic/typing.py#L218

Think this is relevent - just testing

Update: Tested this function - seems to be working ok

@mark-todd
Copy link
Contributor Author

Ok think I might have it: https://github.com/pydantic/pydantic/blob/1.10.X-fixes/pydantic/generics.py#L293

In this line the origins are correctly found as List or list, but the difference is that...

List['test'] -> List[ForwardRef('test')]
While
list['test'] -> list['test'], when it should be list[ForwardRef('test')] - which as it turns out are not equivalent. There needs to be something in this replace_args function that converts strings to ForwardRefs

@mark-todd
Copy link
Contributor Author

This fork fixes the bug - it unfortunately also breaks one of the tests for discriminated union - not sure how to get round this yet:
https://github.com/mark-todd/pydantic/tree/generics-forward-ref-fix

@lig
Copy link
Contributor

lig commented Sep 18, 2023

This works in Pydantic v2.3.0.

from typing import Generic, List, TypeVar

from pydantic import BaseModel

ChangingType = TypeVar("ChangingType")

class Outer(BaseModel, Generic[ChangingType]):
    fields_: list[ChangingType] # Swapping list -> List on this line fixes the bug

outer = Outer["Inner"]
class Inner(BaseModel):
    new: list[outer]

out = Inner.model_validate({"new": [{"fields_": []}]})

print(out)
# new=[Outer[Inner](fields_=[])]

There is even no need to use BaseModel.model_rebuild() method (former update_forward_refs).

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 unconfirmed Bug not yet confirmed as valid/applicable
Projects
None yet
Development

No branches or pull requests

2 participants