Pydantic: Nested Models with optional members fail on 1.6.1

Created on 17 Jul 2020  路  7Comments  路  Source: samuelcolvin/pydantic

Bug

Defining a model with Optional nested recursive submodels is throwing a required value error when validating the model. The nested models also contain nested submodels that container other recursive models potentially. This works on 1.6.0 and all versions prior that we have been using the last year. It only breaks when we move to version 1.6.1

Here is a sample of the model that we are using with basic one letter named attributes.

class Spec(BaseModel):
    spec_fields: List[str] = Field(..., alias="fields")
    filter: Optional[str]
    sort: Optional[str]


class PSpec(Spec):
    g: Optional[GSpec]

class GSpec(Spec):
    p: Optional[PSpec]

class Filter(BaseModel):
    g: Optional[GSpec]
    p: Optional[PSpec]

Trying with the following values will generate an error

        data = {"p": {"sort": "some_field:asc", "fields": []}}
        result = Filter(**data)

The error generated is:

E   pydantic.error_wrappers.ValidationError: 1 validation error for Filter
E   p -> g
E     field required (type=value_error.missing)

It seems to be disregarding the Optional attribute for GSpec field in the nested PSpec model.
Same code on 1.6.0 works as expected.

Thanks!

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

pydantic version: 1.6.1
            pydantic compiled: True
                 install path: /Users/..../.local/share/virtualenvs/..../lib/python3.7/site-packages/pydantic
               python version: 3.7.3 (default, Mar 27 2019, 09:23:15)  [Clang 10.0.1 (clang-1001.0.46.3)]
                     platform: Darwin-19.5.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions', 'email-validator']
import pydantic

...
bug

Most helpful comment

Hi @roycollins
I can't reproduce your issue with the code sample. I reproduced the (almost) same environment

             pydantic version: 1.6.1
            pydantic compiled: True
                 install path: /Users/.../.pyenv/versions/3.7.7/envs/.../lib/python3.7/site-packages/pydantic
               python version: 3.7.7 (default, Mar 18 2020, 12:01:21)  [Clang 10.0.1 (clang-1001.0.46.4)]
                     platform: Darwin-18.7.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions', 'email-validator']

and just added PSpec.update_forward_refs() as it is impossible to run without it.

from pydantic import BaseModel, Field
from typing import List, Optional


class Spec(BaseModel):
    spec_fields: List[str] = Field(..., alias="fields")
    filter: Optional[str]
    sort: Optional[str]


class PSpec(Spec):
    g: Optional['GSpec']

class GSpec(Spec):
    p: Optional[PSpec]

PSpec.update_forward_refs()

class Filter(BaseModel):
    g: Optional[GSpec]
    p: Optional[PSpec]


data = {"p": {"sort": "some_field:asc", "fields": []}}
print(repr(Filter(**data)))

output

Filter(g=None, p=PSpec(spec_fields=[], filter=None, sort='some_field:asc', g=None))

All 7 comments

Hi @roycollins
I can't reproduce your issue with the code sample. I reproduced the (almost) same environment

             pydantic version: 1.6.1
            pydantic compiled: True
                 install path: /Users/.../.pyenv/versions/3.7.7/envs/.../lib/python3.7/site-packages/pydantic
               python version: 3.7.7 (default, Mar 18 2020, 12:01:21)  [Clang 10.0.1 (clang-1001.0.46.4)]
                     platform: Darwin-18.7.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions', 'email-validator']

and just added PSpec.update_forward_refs() as it is impossible to run without it.

from pydantic import BaseModel, Field
from typing import List, Optional


class Spec(BaseModel):
    spec_fields: List[str] = Field(..., alias="fields")
    filter: Optional[str]
    sort: Optional[str]


class PSpec(Spec):
    g: Optional['GSpec']

class GSpec(Spec):
    p: Optional[PSpec]

PSpec.update_forward_refs()

class Filter(BaseModel):
    g: Optional[GSpec]
    p: Optional[PSpec]


data = {"p": {"sort": "some_field:asc", "fields": []}}
print(repr(Filter(**data)))

output

Filter(g=None, p=PSpec(spec_fields=[], filter=None, sort='some_field:asc', g=None))

Here is some more details to make it work. It seems if I import the models from another file then I get the error. If I run the models in the same file then I dont have any issues. Here are the 2 files that I am using.

If I run the files with pydantic 1.6.1 it throws the error above. If I downgrade to the 1.6.0 version then it works.

Archive.zip

Hi @roycollins
I just tried quickly and it indeed crashes with v1.6.1 and not with v1.6. Moving some code of ModelField.prepare into a dedicated method breaks the code. But I'm not convinced your code is supposed to work when you use GSpec without quotes to make as it is a forward ref.

from __future__ import annotations
from pydantic import BaseModel, Field
from typing import List, Optional


class Spec(BaseModel):
    spec_fields: List[str] = Field(..., alias="fields")
    filter: Optional[str]
    sort: Optional[str]


class PSpec(Spec):
    g: Optional[GSpec]


class GSpec(Spec):
    p: Optional[PSpec]

PSpec.update_forward_refs()

class Filter(BaseModel):
    g: Optional[GSpec]
    p: Optional[PSpec]


data = {"p": {"sort": "some_field:asc", "fields": []}}
print(repr(Filter(**data)))

this crashes indeed!
And in fact when I remove the from __future__ import annotations, the error changes to be

NameError: name 'GSpec' is not defined

If you change class PSpec into

class PSpec(Spec):
    g: Optional['GSpec']

can you confirm it works as expected please?

Yes removing the __future__ for delayed annotations does indeed work if I define the types as a string. I am perfectly fine converting the type annotations to strings to make it work.

I was more concerned that it stopped working on 1.6.1 when it worked on 1.6.0. It does seem to be a change with the way nested models are instantiated when they are optional only. If they are not optional it seems to work.

There is supposed to be support for python 3.7 delayed annotations per the last block of the docs:
https://docs.google.com/document/d/1jbHNCcAyMC29-2ooBcLz0ROaA0TOfPX-JL2e9klZJEE/edit#
I would be willing to update that part of the docs if it is not meant to support Optional.

Thanks for taking the time to help get to the bottom of this issue.

You can keep the __future__ import. Just have to change the type into string as it doesn't exist yet.
I'm more concerned on "why it used to work before?" personally x)

EDIT: I understood the "regression"!

@roycollins I opened #1752. Feel free to try it out and comment.
Sorry for the regression.

Thank you for the quick response. I tested out the commit and it works perfectly!!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cdeil picture cdeil  路  3Comments

krzysieqq picture krzysieqq  路  3Comments

sbv-trueenergy picture sbv-trueenergy  路  3Comments

timonbimon picture timonbimon  路  3Comments

vvoody picture vvoody  路  3Comments