default_factory being run prior to object init

[xlotlu]

Bug

             pydantic version: 1.5.1

default_factory gets run twice before any object instantiation. This is obvious for callables with side-effects:

>>> from pydantic import BaseModel, Field
>>> class Seq():
...     def __init__(self):
...         self.v = 0 
...     def __call__(self):
...         self.v += 1
...         return self.v
...   
>>> class MyModel(BaseModel):
...     id: int = Field(default_factory=Seq())
...   
>>> MyModel()
MyModel(id=3)

[samuelcolvin]

I'm not sure I would call this a bug.

Since it'll be consistent, you should be able to just start with v = -2 or whatever - I know it looks odd but as long as you include a comment I think it should be fine.

I can't think of another safe workaround.

[bharel]

@samuelcolvin Unfortunately our team also encountered this bug. Starting with v = -2 is just not possible in some cases, for factories that cause side effects e.g. writing to files, loading env vars which don't exist as of yet...

[samuelcolvin]

@bharel if you're writing to a file or messing with the environment in a factory function you're doing something very wrong.

[aviramha]

What is the purpose of running default factory function on class creation?

[bharel]

@samuelcolvin I'm sorry but I don't think this is wrong:

class RedisSettings(BaseSettings):
    host: str = Field(..., env='RedisHost')
    
class Settings(BaseSettings):
    redis: RedisSettings = Field(default_factory=RedisSettings)

All I'm using is pydantic, and it'll fail on import time if the env doesn't exist.
Which it doesn't while unittesting etc.

[samuelcolvin]

What is the purpose of running default factory function on class creation?

See #1504, it's called to get the field type - I think this is fixed on #1504 by making type annotations required for fields with a factory.

But I haven't looked through #1504 completely I think it might require some more work.

@samuelcolvin I'm sorry but I don't think this is wrong:

class RedisSettings(BaseSettings):
    host: str = Field(..., env='RedisHost')
    
class Settings(BaseSettings):
    redis: RedisSettings = Field(default_factory=RedisSettings)

All I'm using is pydantic, and it'll fail on import time if the env doesn't exist.
Which it doesn't while unittesting etc.

This is an interesting use case, I'll think more about how best to work around the problem or solve it completely.

[aviramha]

@samuelcolvin
Have you had the chance to think about it? If you have an idea but no time to implement it I'll be happy to send a PR.

[samuelcolvin]

see #1504, is there anything you think is wrong with that? It seems a good fix to me.

[aviramha]

@samuelcolvin I'm asking regarding the example @bharel posted. Would that PR fix that also? (i.e will not call the factory on class definition?)

[samuelcolvin]

Why don't you try the PR and find out? Then you could report back here how it works or comment on the PR with suggestions.

[aviramha]

@samuelcolvin sorry, I think I read in the PR that it makes it run only once (I assumed it meant instead of twice, but actually it meant only on initialization, not class deifintion).
I tested it now and works as expected!
I'll follow the PR.