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

Model that inherits from a model in another file cannot evaluate ForwardRefs of inherited fields #4902

Closed
5 of 15 tasks
zachkirsch opened this issue Jan 3, 2023 · 2 comments
Closed
5 of 15 tasks
Assignees
Labels
bug V1 Bug related to Pydantic V1.X bug V2 Bug related to Pydantic V2

Comments

@zachkirsch
Copy link
Contributor

zachkirsch commented Jan 3, 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 very similar to #3666 but for regular pydantic models, not dataclasses.

Repro repo here: https://github.com/zachkirsch/pydantic-circular-references-repro

Baz.update_forward_refs() fails with the following error:

Traceback (most recent call last):
  File "src/main.py", line 1, in <module>
    from circular_pydantic_models.bar import Bar
  File "/private/var/folders/zb/sncxfc0n1y96dwgkk2phgb9w0000gn/T/pytest-of-zachkirsch/pytest-39/test_ir_pydantic_model0/repro2/src/circular_pydantic_models/bar.py", line 9, in <module>
    from .foo import Foo
  File "/private/var/folders/zb/sncxfc0n1y96dwgkk2phgb9w0000gn/T/pytest-of-zachkirsch/pytest-39/test_ir_pydantic_model0/repro2/src/circular_pydantic_models/foo.py", line 4, in <module>
    from .baz import Baz
  File "/private/var/folders/zb/sncxfc0n1y96dwgkk2phgb9w0000gn/T/pytest-of-zachkirsch/pytest-39/test_ir_pydantic_model0/repro2/src/circular_pydantic_models/baz.py", line 18, in <module>
    Baz.update_forward_refs()
  File "pydantic/main.py", line 816, in pydantic.main.BaseModel.update_forward_refs
  File "pydantic/typing.py", line 553, in pydantic.typing.update_model_forward_refs
    def_mod = sys._getframe(1).f_globals.get('__name__', '__main__')  # for pickling
  File "pydantic/typing.py", line 519, in pydantic.typing.update_field_forward_refs
    in all type variables.
  File "pydantic/typing.py", line 58, in pydantic.typing.evaluate_forwardref
    'MappingView',
  File "/Users/zachkirsch/.pyenv/versions/3.7.13/lib/python3.7/typing.py", line 467, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'Foo' is not defined

In the above example, Baz inherits a field bar_field: "Foo" from its superclass Bar. Baz.update_forward_refs() tries to resolve this forward ref by looking at module.__dict__, but Foo is not in scope in baz.py, so it throws.

Pydantic uses the module.__dict__ of the model's file to resolve forward references. But for inherited fields, I believe it would make sense for Pydantic to instead use the model.__dict__ of the module containing the field.

Example Code

# foo.py
import typing
import pydantic

from .baz import Baz

class Foo(pydantic.BaseModel):
    foo_field: typing.Optional[Baz]


# bar.py
import pydantic

class Bar(pydantic.BaseModel):
    bar_field: "Foo"

from .foo import Foo

Bar.update_forward_refs()


# baz.py
import pydantic

from .bar import Bar 

# we're inherited from Bar
class Baz(Bar):
    ...

Baz.update_forward_refs()

Python, Pydantic & OS Version

pydantic version: 1.10.4
pydantic compiled: True
install path: /Users/zachkirsch/Library/Caches/pypoetry/virtualenvs/pydantic-circular-references-repro-XJSSiZt5-py3.7/lib/python3.7/site-packages/pydantic
python version: 3.7.13 (default, Oct 12 2022, 21:19:21)  [Clang 12.0.5 (clang-1205.0.22.11)]
platform: Darwin-22.1.0-x86_64-i386-64bit
optional deps. installed: ['typing-extensions']

Affected Components

@zachkirsch zachkirsch added bug V1 Bug related to Pydantic V1.X unconfirmed Bug not yet confirmed as valid/applicable labels Jan 3, 2023
@zachkirsch zachkirsch changed the title Models with a base class in a different model cannot evaluate ForwardRefs of inherited fields Model that inherits from a model in another file cannot evaluate ForwardRefs of inherited fields Jan 3, 2023
@zachkirsch
Copy link
Contributor Author

zachkirsch commented Jan 3, 2023

I'd love to make a PR to fix this - any suggestions on where to start @samuelcolvin? In particular, would like to start by running a failing test and stepping through with a debugger.

@Kludex Kludex added bug V2 Bug related to Pydantic V2 v2-reviewed and removed unconfirmed Bug not yet confirmed as valid/applicable labels May 1, 2023
@dmontagu
Copy link
Contributor

I think this is now fixed, through the use of the _types_namespace argument. I couldn't get it to work with your example above because python gave me a circular import error, but with the following tweaks I was able to make it work:

# pydantic/dev/foo.py:
import typing
import pydantic

from .baz import Baz


class Foo(pydantic.BaseModel):
    foo_field: typing.Optional[Baz]

# pydantic/dev/bar.py:
from typing import TYPE_CHECKING

import pydantic

if TYPE_CHECKING:
    from .foo import Foo

class Bar(pydantic.BaseModel):
    bar_field: "Foo"

# pydantic/dev/baz.py:
from .bar import Bar

# we're inherited from Bar
class Baz(Bar):
    ...

# scratch.py:
from pydantic.dev.bar import Bar
from pydantic.dev.baz import Baz
from pydantic.dev.foo import Foo

Bar.model_rebuild(_types_namespace={'Foo': Foo})
Baz.model_rebuild(_types_namespace={'Foo': Foo})

With this, I can run scratch.py without errors. While this might not be exactly how you'd want to organize your code, I think this means the issue is addressed; if not, please let us know.

Also, I'll just note that the reason why the argument is _types_namespace instead of types_namespace is that this is such a rare scenario that I wanted to discourage the use of the argument when not necessary. However, we also made it so that rebuilds happen "automatically" in most cases, so now that you generally don't need to call the model_rebuild method manually in most scenarios, I think it might make sense to change it back to types_namespace now.

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 bug V2 Bug related to Pydantic V2
Projects
None yet
Development

No branches or pull requests

3 participants