The flag allow_population_by_field_name is not recognized

[islam-aymann]

I have two models User, Review and their ModelFactorys.

from pydantic import BaseModel, Field

from pydantic_factories import ModelFactory


class User(BaseModel):
    login: str


class Review(BaseModel):
    body: str
    author: User = Field(alias="user")

    class Config:
        allow_population_by_field_name = True


class UserFactory(ModelFactory):
    __model__ = User


class ReviewFactory(ModelFactory):
    __model__ = Review


if __name__ == '__main__':
    author: User = UserFactory.build(login="me")

    review: Review = ReviewFactory.build(author=author)
    assert id(author) != id(review.author)
    assert review.author.login != author.login

    review: Review = ReviewFactory.build(user=author)
    assert id(author) != id(review.author)  # 🙄 why?
    assert review.author.login == author.login

*note: all assertion are successful

Review model has the allow_population_by_field_name flag set to True which means
Review model should be able to accept both author and user attributes to
populate the User model, however it's not recognized on building the instance and
new instance gets created.

I also noticed that new object was created on supplying a valid User instance to the ReviewFactory!!
see the why line

[Goldziher]

Well, I wouldn't expect this to work - it doesn't exist :). Also, not in the docsö.

What is this flag supposed to achieve exactly?

[islam-aymann]

The flag gives the Model the ability to accept the data in both the field name and the alias.

so you can do:

data = {
    "body": "lorem ipsum ..."
    "author": {
        "login": "username",
        ...
    }
}

obj = Review(**data)

same as:

data = {
    "body": "lorem ipsum ..."
    "user": {
        "login": "username",
        ...
    }
}

obj = Review(**data)

[Goldziher]

oh, you mean the pydantic config. I see. So, this library doesn't read the pydantic model config, but rather the configuration of individual fields.

I'm not sure how desirable this behaviour is - @lindycoder , thoughts?

@islam-main how would you suggest implementing this?

[lindycoder]

Yes i'm missing this behaviour actually, wanted to propose an update but i'm short on time.
i was thinking a __by_alias__. true/false that fits the contract of pydantic's .dict(by_alias=False) and .json(by_alias=False) functions... and so .build() would either use the alias or the field name, i don't see a point in supporting both in the same .build() call

Would default to True for backward compatibility

That new config would turn off this if
https://github.com/Goldziher/pydantic-factories/blob/main/pydantic_factories/factory.py#L489

            if model_field.alias:
                field_name = model_field.alias

[Goldziher]

Ok. Then a PR is welcome here 😃. I'll add the label.

[islam-aymann]

@sondrelg @Goldziher Why wouldn't we support both (name and alias) and implement the same behavior of Pydantic itself?
and I guess maybe we should extract the flag from the Config class, not letting the user to duplicate information?

[lindycoder]

It's true that if you try to make the factory work on field name but the model doesn't have the flag then it will fail. That's a good argument for reading the model's config.

[Goldziher]

Implementation should support both I guess. I mean, both the field name and alias.

[islam-aymann]

I am also short on time but I think I might be able to open a PR in a week or something to initiate Pydantic Model Config support.

[islam-aymann]

I'll start with this issue flag allow_population_by_field_name as it's very popular on GitHub.

[mrkovalchuk]

@islam-main @Goldziher #43 this is it?

I have the same issues. If this PR looks fine, I would be glad to merge it as soon as possible

[islam-aymann]

@mrkovalchuk Yes it's, I'll check it.

[Goldziher]

Thank you for contributing. There are comments on the PR.

[Goldziher]

Thanks a lot for contributing @mrkovalchuk - v1.2.9 is been released with your contribution.