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

Non-primitive type (object) defaults are shared between instances #154

Closed
atleta opened this Issue Apr 9, 2018 · 3 comments

Comments

2 participants
@atleta

atleta commented Apr 9, 2018

Hi,

If you use any non-primitive type as a default and create a model so that the default is used (i.e. not specifying the field in the serialized data) then you will end up having the same instance across all the instances (that were initized with the default). Now, in hindshight it's not surprising, because this is what happens when you have such a default for a function parameter, but the documentation is misleading as it contains an example suggesting you could do this:

class User(BaseModel):
    id: int
    name = 'John Doe'
    signup_ts: datetime = None
    friends: List[int] = []

While if now you create two users:

u1, u2 = [User(id=i) for i in range(2)]

You'll have u1.friends is u2.friends. This one is easy to step aside by always providing a friends list, but in my case I wanted to use a defaultdict, which, it seems can be fixed with validators (using always=True, and also copying the values from the dict created by pydantic) but it would be nice to be able to use factory functions for a more declarative style. E.g. if you know FactoryBoy, something like that. (Though, probably one should still have a converting validator if there is actually data for that field.)

But at least the documentation should be fixed.

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Apr 9, 2018

I guess the correct solution is to duplicate the default for each instance such that u1.friends is not u2.frineds.

I'll work on a PR when I have the time.

samuelcolvin added a commit that referenced this issue Jun 4, 2018

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Jun 4, 2018

Sorry for the delay. Fixed in #192, let me know if that works for you.

samuelcolvin added a commit that referenced this issue Jun 4, 2018

@atleta

This comment has been minimized.

atleta commented Jun 6, 2018

Thanks, looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment