litestar-org / polyfactory

Simple and powerful factories for mock data generation
https://polyfactory.litestar.dev/
MIT License
988 stars 78 forks source link

fix: pydantic factory_use_construct is not propagated to the nested #549

Closed Reskov closed 2 months ago

Reskov commented 3 months ago

Description

Closes

Fixes #548

guacs commented 3 months ago

looks like that there is still a possibility that factory_use_construct will not be propagated to the constrained types

I didn't understand what you meant here. Why would we need to pass that to constrained types? We can't use factory_use_construct for constrained types (i.e. the more primitive types like int, float, datetime etc.) anyway. We can only use it for pydantic models, but we can't have constraints on them as a whole anyway.

Reskov commented 3 months ago

I didn't understand what you meant here. Why would we need to pass that to constrained types? We can't use factory_use_construct for constrained types (i.e. the more primitive types like int, float, datetime etc.) anyway. We can only use it for pydantic models, but we can't have constraints on them as a whole anyway.

Pydantic supports collection constrained types, where we can pass arbitrary type, ex conlist https://docs.pydantic.dev/latest/api/types/#pydantic.types.conlist

 class Parent(BaseModel):
        child: conlist(Child, min_length=5, max_length=10)

As I understand, this will fail, because https://github.com/litestar-org/polyfactory/blob/740fd345685df6b313f8a4e12e041129b4dcd2e1/polyfactory/value_generators/constrained_collections.py#L47 do not operate with an extra build args and calls get_field_value just with field meta.

guacs commented 3 months ago

I didn't understand what you meant here. Why would we need to pass that to constrained types? We can't use factory_use_construct for constrained types (i.e. the more primitive types like int, float, datetime etc.) anyway. We can only use it for pydantic models, but we can't have constraints on them as a whole anyway.

Pydantic supports collection constrained types, where we can pass arbitrary type, ex conlist https://docs.pydantic.dev/latest/api/types/#pydantic.types.conlist

 class Parent(BaseModel):
        child: conlist(Child, min_length=5, max_length=10)

As I understand, this will fail, because

https://github.com/litestar-org/polyfactory/blob/740fd345685df6b313f8a4e12e041129b4dcd2e1/polyfactory/value_generators/constrained_collections.py#L47

do not operate with an extra build args and calls get_field_value just with field meta.

Aah of course! That makes sense. Hm...that's going to take some refactoring in order to be able to handle that. We can do that in another PR or in this PR itself. I'm fine with either. Whichever you prefer, @Reskov.

Reskov commented 3 months ago

@guacs Hello, looks like I've done with refactoring, please take a look.

There were some other caveats:

guacs commented 2 months ago

@all-contributors add @Reskov for code

allcontributors[bot] commented 2 months ago

@guacs

I've put up a pull request to add @Reskov! :tada:

Reskov commented 2 months ago

@guacs @adhtruong Sorry for doing this, but I just realized that I broke previous behavior (generation of the unique set for the constrained type). I've misused the for loop instead of previous while, which is not correct since it has no retries in case of duplicate of the set generated value.

P.S. That's why test_collection_length_with_list became flaky

github-actions[bot] commented 2 months ago

Documentation preview will be available shortly at https://litestar-org.github.io/polyfactory-docs-preview/549