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

Some recursive models do not require update_forward_refs and silently behave incorrectly #1201

Closed
dmontagu opened this issue Jan 31, 2020 · 3 comments · Fixed by #2221
Closed
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@dmontagu
Copy link
Contributor

Bug

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

             pydantic version: 1.4
            pydantic compiled: False
                 install path: /Users/dmontague/Programming/oss/pydantic/pydantic
               python version: 3.7.4 (default, Oct 17 2019, 23:32:33)  [Clang 11.0.0 (clang-1100.0.33.8)]
                     platform: Darwin-19.0.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions', 'email-validator', 'devtools']

This test passes on master, but it should not:

from typing import Optional, Tuple

from pydantic import BaseModel


def test_parse_nested_tuple():
    class NestedTuple(BaseModel):
        x: Tuple[int, Optional["NestedTuple"]]

    obj = NestedTuple.parse_obj({'x': ('1', {'x': ('2', {'x': ('3', None)})})})
    assert obj.dict() == {'x': (1, {'x': ('2', {'x': ('3', None)})})}
    # Note above: the 2 and 3 did not get parsed into ints !!

    NestedTuple.update_forward_refs()
    obj = NestedTuple.parse_obj({'x': ('1', {'x': ('2', {'x': ('3', None)})})})
    assert obj.dict() == {'x': (1, {'x': (2, {'x': (3, None)})})}

Presumably we should either get an error at some point prior to the NestedRootTuple.update_forward_refs() call, or the two calls to obj.dict() should generate the same output.

I'm guessing this is related to the field being Optional, but I'm not sure. I discovered it while working on #1200 .

@dmontagu dmontagu added the bug V1 Bug related to Pydantic V1.X label Jan 31, 2020
@samuelcolvin
Copy link
Member

yes, very likely related to Optional[]. What happens if you used List['NestedTuple']?

@dmontagu
Copy link
Contributor Author

dmontagu commented Feb 2, 2020

Looks like it fails in the same way. This test passes:

from typing import List, Tuple

from pydantic import BaseModel


def test_parse_nested_tuple():
    class NestedTuple(BaseModel):
        x: Tuple[int, List["NestedTuple"]]

    obj = NestedTuple.parse_obj({'x': ('1', [{'x': ('2', [{'x': ('3', [])}])}])})
    assert obj.dict() == {'x': (1, [{'x': ('2', [{'x': ('3', [])}])}])}

    # Note above: the 2 and 3 did not get parsed into ints !!

    NestedTuple.update_forward_refs()
    obj = NestedTuple.parse_obj({'x': ('1', [{'x': ('2', [{'x': ('3', [])}])}])})
    assert obj.dict() == {'x': (1, [{'x': (2, [{'x': (3, [])}])}])}

@matanagasauce
Copy link

Looks like a problem of checking ForwardRef. ForwardRef is now checked only in validate_model but not in ModelField.validate, so an error is raised only when ModelField.type_ is calculated ForwardRef. But this doesn't happen to Tuple.
Which means the following test passes

class NestedTuple(BaseModel):
    x: Tuple["NestedTuple"]
obj = NestedTuple.parse_obj({'x': (AnyObject, )})
assert obj.x[0] is AnyObject

Maybe we can move the checking into ModelField to solve this?

PrettyWood added a commit to PrettyWood/pydantic that referenced this issue Dec 17, 2020
PrettyWood added a commit to PrettyWood/pydantic that referenced this issue Dec 24, 2020
PrettyWood added a commit to PrettyWood/pydantic that referenced this issue Dec 24, 2020
samuelcolvin added a commit that referenced this issue Jan 2, 2021
* fix: force need of `update_forward_refs` in recursive models

closes #1201

* fix: lint issue (false positive)

PyCQA/pyflakes#567

* chore: avoid if statement for coverage

* fix linting

Co-authored-by: Samuel Colvin <s@muelcolvin.com>
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.

3 participants