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

NamedTuple doesn't resolve postponed annotations #2760

Closed
3 tasks done
jameysharp opened this issue May 7, 2021 · 7 comments · Fixed by #2761
Closed
3 tasks done

NamedTuple doesn't resolve postponed annotations #2760

jameysharp opened this issue May 7, 2021 · 7 comments · Fixed by #2761
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@jameysharp
Copy link
Contributor

Checks

  • I added a descriptive title to this issue
  • I have searched (google, github) for similar issues and couldn't find anything
  • I have read and followed the docs and still think this is a bug

Bug

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

             pydantic version: 1.8.1
            pydantic compiled: False
                 install path: /nix/store/1f6nk37c7df54cm8b3rlnn6pidaa3mrp-python3.8-pydantic-1.8.1/lib/python3.8/site-packages/pydantic
               python version: 3.8.8 (default, Feb 19 2021, 11:04:50)  [GCC 9.3.0]
                     platform: Linux-5.4.114-x86_64-with
     optional deps. installed: ['typing-extensions']

Here's the smallest program I could come up with that fails under from __future__ import annotations but works without it:

from __future__ import annotations

from pydantic import BaseModel, PositiveInt
from typing import NamedTuple

class Tup(NamedTuple):
    v: PositiveInt

class Mod(BaseModel):
    t: Tup

print(Mod.parse_obj({"t": [3]}))

The call to parse_obj raises this exception: pydantic.errors.ConfigError: field "v" not yet prepared so type is still a ForwardRef, you might need to call Tup.update_forward_refs(). But there is no update_forward_refs on NamedTuple instances.

Just using a base int in the NamedTuple doesn't exhibit the same problem, but of course it also doesn't check the constraints I want.

My current workaround is to just not use from __future__ import annotations in the module where I declare and use the NamedTuple, but I was hoping to use the new style consistently throughout the project.

@jameysharp jameysharp added the bug V1 Bug related to Pydantic V1.X label May 7, 2021
@PrettyWood
Copy link
Member

PrettyWood commented May 8, 2021

Hi @jameysharp
Yep you're right but the workaround is easy (just not documented). Just like dataclass or TypedDict, pydantic will attach a BaseModel to the main class under __pydantic_model__.
So you should be able to make it work by running
Tup.__pydantic_model__.update_forward_refs()
(I'm traveling so can't check on my phone for sure)

@jameysharp
Copy link
Contributor Author

Oh, that's interesting. It doesn't work, but now I know things I didn't before. 😁

If I call Tup.__pydantic_model__.update_forward_refs() immediately after declaring the NamedTuple, it fails with AttributeError: type object 'Tup' has no attribute '__pydantic_model__', which seems reasonable.

If I call it after declaring the Pydantic model, then I get a different error (and a similar result in the real application I stripped this down from): NameError: name 'PositiveInt' is not defined

So it doesn't seem to be resolving the names using the correct global scope. My current hypothesis is that the same thing is happening at model definition time, but the error is hidden then because it's indistinguishable from a forward reference, even though this isn't actually a forward reference.

By contrast, declaring the NamedTuple field's type as "int" without future annotations, or as int with future annotations, works fine even without an update_forward_refs call, I assume because that name is bound in any scope.

So I think there are multiple bugs here:

  1. If people are expected to use Foo.__pydantic_model__.update_forward_refs(), then the ConfigError should say that instead of Foo.update_forward_refs(), and the documentation should cover that case.

  2. Maybe the implicit __pydantic_model__ needs to be tied to the same module as the class which it wraps, somehow?

@PrettyWood
Copy link
Member

Yes it is added of course by pydantic once used in a BaseModel. A plain named tuple does not have this attribute so you need to call it after declaring your model.
And for the context you're right locals are not the same.
So in your case it should work with something like
Tup.__pydantic_model__.update_forward_refs(PositiveInt=PositiveInt)
We can (and should?) forward locals and stuff to resolve probably forward refs but it probably will never be perfect. So some doc and a better error message are probably a good idea as well

@jameysharp
Copy link
Contributor Author

But PositiveInt isn't a local here, it's imported in the module at global scope. The Postponed Annotations section of the documentation says that should work. That's why I think the synthesized __pydantic_model__ is referencing the wrong module.__dict__.

@PrettyWood
Copy link
Member

PrettyWood commented May 8, 2021

Yep I know I meant "we can forward more than just the module" sorry if I was not clear.
As I told you I'm on my phone but if you want to change your local pydantic you can try to change https://github.com/samuelcolvin/pydantic/blob/master/pydantic/validators.py#L561 and set __module__ like it's done for dataclass https://github.com/samuelcolvin/pydantic/blob/master/pydantic/dataclasses.py#L186
If you want to open a fix you're more than welcome 😉
If not I'll have a look and open a fix end of next week! Thanks for reporting!

jameysharp added a commit to jameysharp/pydantic that referenced this issue May 8, 2021
Thanks to @PrettyWood for pointing me to the right place to fix this!

Since I was told that both NamedTuple and TypedDict use the same
__pydantic_model__ machinery that dataclasses do, I checked and found
that TypedDict had the same bug, and fixed that too.

Tests for both issues are included, which fail without the associated
fixes being applied.
jameysharp added a commit to jameysharp/pydantic that referenced this issue May 9, 2021
Thanks to @PrettyWood for pointing me to the right place to fix this!

Since I was told that both NamedTuple and TypedDict use the same
__pydantic_model__ machinery that dataclasses do, I checked and found
that TypedDict had the same bug, and fixed that too.

Tests for both issues are included, which fail without the associated
fixes being applied.
@samuelcolvin
Copy link
Member

Thanks for reporting and for the PR, I'll review it now.

On the general issue of postponed annotations, #2678 is relevant. It looks like from __future__ import annotations won't just become the default as we had previously expected.

@phemmer
Copy link

phemmer commented Jul 9, 2021

Just ran into this as well. The PR has been sitting for a couple months now. Any chance we can get that merged?

PrettyWood pushed a commit that referenced this issue Sep 4, 2021
Thanks to @PrettyWood for pointing me to the right place to fix this!

Since I was told that both NamedTuple and TypedDict use the same
__pydantic_model__ machinery that dataclasses do, I checked and found
that TypedDict had the same bug, and fixed that too.

Tests for both issues are included, which fail without the associated
fixes being applied.
jpribyl pushed a commit to liquet-ai/pydantic that referenced this issue Oct 7, 2021
Thanks to @PrettyWood for pointing me to the right place to fix this!

Since I was told that both NamedTuple and TypedDict use the same
__pydantic_model__ machinery that dataclasses do, I checked and found
that TypedDict had the same bug, and fixed that too.

Tests for both issues are included, which fail without the associated
fixes being applied.
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