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

BaseModel.construct returns instances with None assigned to fields with a default_factory #1732

Closed
zoopp opened this issue Jul 17, 2020 · 5 comments · Fixed by #1755
Closed
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@zoopp
Copy link

zoopp commented Jul 17, 2020

Bug

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

             pydantic version: 1.6.1
            pydantic compiled: True
                 install path: /home/mihai/.virtualenvs/pydantic/lib/python3.8/site-packages/pydantic
               python version: 3.8.3 (default, May 17 2020, 18:15:42)  [GCC 10.1.0]
                     platform: Linux-5.7.7-1-ck-x86_64-with-glibc2.2.5
     optional deps. installed: ['devtools']
from pydantic import BaseModel, Field
from typing import List

class Model(BaseModel):
    foo: List[int] = Field(default_factory=list)
    bar: str = 'Baz'

print(Model.construct()) 

# in pydantic   1.5.1 the result is: foo=[] bar='Baz'
# in pydantic  ^1.6.0 the result is: foo=None bar='Baz'

Hello, I'm using pydantic in an application and after updating to v1.6.1 from v1.5.1 I came across this situation which results in a couple of failures.

From the changelog and documentation it's not clear if this change was intended or not.

@zoopp zoopp added the bug V1 Bug related to Pydantic V1.X label Jul 17, 2020
@zoopp zoopp changed the title BaseModel.construct returns instances with None assigned to fields with a default_factory BaseModel.construct returns instances with None assigned to fields with a default_factory Jul 17, 2020
@PrettyWood
Copy link
Member

Hi @zoopp,
Well we made some changes on default_factory to avoid calling it for nothing and avoid some side effects in #1504 and #1712
But this line changes the default value that used to be set in v1.5.1 hence the None.
You could just use Model() instead of Model.construct() to fix this issue in the meantime.
I don't know what we really want in term of behaviour to

  • avoid calling default_factory when not needed
  • still have meaningful default values with construct

@zoopp
Copy link
Author

zoopp commented Jul 20, 2020

Thank you for the reply @PrettyWood,

It's okay, I can work around the issue until a consensus is reached on what the behavior should be. Speaking from my point of view, or more generally from a user of the library's point of view, I would say it would make sense that if construct initializes fields with default values then it should initialize fields with default_factory as well.

Would it make sense to call the factories inside the construct method as needed?

@PrettyWood
Copy link
Member

@zoopp I'll try to work on this bug soon. I agree the behaviour should be the same as calling Model.__init__ in term of fields and set values. There is a very easy fix but it's quite ugly and I'd like to take a bit more time to see if we can find a cleaner solution

@PrettyWood
Copy link
Member

@zoopp I opened a PR with a proposal to support default_factory properly. Your feedback is welcome :)

@zoopp
Copy link
Author

zoopp commented Jul 27, 2020

I looked over the PR and from my point of view it seems well done. I've tested your fix in my application as well and things work without any issues.

Awesome job! 👍

samuelcolvin pushed a commit that referenced this issue Oct 25, 2020
* feat: support `default_factory` with `BaseModel.construct`

closes #1732

* refactor: remove __field_defaults__

* docs: update change with `__field_defaults__` deletion
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.

2 participants