litestar-org
Enhancement: have Trait/HiddenField only there to feed the post_generate of other fields
Summary
Hi !
Not sure if Documentation Request or Enhancement, took the second one as I did not found it...
I want to use Faker's local_latlng to feed several fields, latitude, longitude, city.
I think the way to go is to use post_generate, however now I don't see a better way to do so then to call the function X times and then access the item of the tuple returned by the function by position.
I think it would be cleaner if PolyFactory was supporting some kind of HiddenField (maybe all fields prefixed by __ ?) to get the value, store the tuple as attribute there, use it to feed other fields, but then never reflect it in model_dump() or other instance logic.
I may have missed a functionality somewhere else...
Basic Example
No response
Drawbacks and Impact
No response
Unresolved questions
No response
Hi @MRigal ,
This isn't explicitly supported currently. Probably worth considering with Traits or Params feature which was mentioned elsewhere.
A workaround could be overriding process_kwargs to avoid multiple calls to local_latlng. Would something like the following work for your use case?
from dataclasses import dataclass
from polyfactory.factories import DataclassFactory
@dataclass
class Model:
lat: float
lng: float
class Factory(DataclassFactory):
__model__ = Model
@classmethod
def process_kwargs(cls, **kwargs: Any) -> dict[str, Any]:
kwargs = super().process_kwargs()
lat, lon, *_ = cls.__faker__.local_latlng()
kwargs.update({
"lat": lat,
"lon": lon,
})
return kwargs
Note this does lose some type safety and name checking so worth considering adding this as a new feature
Hi @adhtruong
Thanks for your answer and your suggestion. It works fine but is a bit too "hidden" to me. I couldn't find any earlier discussion on Trait in the repo unfortunately, would you have something to point me to?
I came up with the following personal basis implementation, which does what I want. What do you think about it? You could steer me towards the direction you would like it to go and I may submit a PR implementing it.
The example belows bases on Pydantic ModelFactory which I'm using.
Note that I've used your suggestion Trait name, but it could as well be a HiddenField in my eyes.
# NB: Creating a separate class here for the check by isinstance and also to not raise while cls._check_declared_fields_exist_in_model()
class Trait(Generic[P, T]):
"""Factory field used to wrap a callable as trait.
The callable will be invoked wehn building the factory itself.
"""
__slots__ = ("fn", "kwargs", "args")
def __init__(self, fn: Callable[P, T], *args: P.args, **kwargs: P.kwargs) -> None:
"""Wrap a callable.
:param fn: A callable to wrap.
:param args: Any args to pass to the callable.
:param kwargs: Any kwargs to pass to the callable.
"""
self.fn: WrappedCallable = {"value": fn}
self.kwargs = kwargs
self.args = args
def to_value(self) -> T:
"""Invoke the callable.
:returns: The output of the callable.
"""
return cast("T", self.fn["value"](*self.args, **self.kwargs))
class TraitModelFactory(ModelFactory[T]):
__is_base_factory__ = True
@classmethod
def build(
cls,
factory_use_construct: bool = False,
**kwargs: Any,
) -> T:
kwargs.update(**cls.traits_to_kwargs())
return super().build(factory_use_construct=factory_use_construct, **kwargs)
@classmethod
def traits_to_kwargs(cls) -> dict[str, Any]:
factory_fields = cls.__dict__.items()
return {
field_name: field_value.to_value()
for field_name, field_value in factory_fields
if isinstance(field_value, Trait)
}
class Factory(TraitModelFactory[Model]):
_trait_category = Trait(ModelFactory.__random__.choice, [1, 2, 3, 4, 5])
@post_generated
@classmethod
def manufacturer(cls, _trait_category: int) -> str:
return f"Polyfactory No {_trait_category}"
Ideally, I would prefer having a Trait (or whatever feature) that would return an object. For example here having something like the following. But I was a bit unsure to modify that much the logic...
_trait_category.id = 1
_trait_category.lat = 52.5
_trait_category.lon = 13.2
_trait_category.city = Laguiole
re-pinging @adhtruong as I'm interested to work out the feature if you give me some guidance :-)
Sorry for the slow reply!
I can't find for conversation for these terms now but believe it comes from factoryboy. Based on their docs, this is probably closer to Param rather than Trait so would favour using that to help ease migration for people coming from that.
Main places think would have to change
- Field to add this as an option or 'lazy' value. I think should support both forms on the class or as a method like with
postgenerated. process_kwargs- this should have awareness ofParamtype and evaluate accordingly.- I think it would be nice to have this overridable as a kwarg passed in via
buildorprocess_kwarg. This allows some flexibility - Similar changes should also be made for
process_kwargs_coverageto support usage that way.
Let me know if need anymore help to start or think should reduce scope of initial implementation! :)
CC: @guacs
Thanks @adhtruong and no prob, we all have different prios at different times!
It gives me a better understanding of what you would like to have. It's indeed a bigger scope, but it makes sense to aim at making it close enough to what is really desired.
Due to a surgery I won't be able to work on it this week, and also next week is uncertain. I still plan to try to implement it afterwards and will keep you updated. :-)
@MRigal @adhtruong I would be interested in helping on this PR. I, too, and wishing for this feature. Let me know if you were able to work on it.
@MRigal In addition to your proposal, there will need to be a way to handle attribute values that are set at build-time attributes that shouldn't get passed to the final object. This is what factory-boy calls a Parameter.
I also had an issue getting the TraitModelFactory to work correctly because the returned dict still contained an entry for _trait_category.
@MRigal @adhtruong just wanted to circle back on this. Let me know how I can be of assistance!
Hi @leverage-analytics !
Unfortunately, I have been completely filled with other work until end of January and then I'll have a month of paternity leave. So no way to start before mid-March at the end. I'd still like to do it, but if you have time to do it before, just go, start something and I may review/comment the PR between other tasks :-)
Great. Agree naming should be consistent with factoryboy on this. Main parts need to change are list here for traits/params but happy to provide any more info or guidance :)
I have a pull request ready for this, but I am having issues getting the pre-commit hooks for mypy and pyright to resolve. Both are raising an issue with the sphinx docs.
docs/conf.py:12: error: Module "sphinx.addnodes" does not explicitly export attribute "document" [attr-defined]
Found 1 error in 1 file (checked 130 source files)
docs/conf.py:12:33 - error: "document" is not exported from module "sphinx.addnodes" (reportPrivateImportUsage)
1 error, 0 warnings, 0 informations
Any help would be appreciated!
Hi @leverage-analytics I'm not skilled at all in mypy+sphinx interactions, so I'm afraid I can't help, but you may publish your branch and open a PR that you mark as Draft to ease feedback, on the errors but also more generally! :-)
Hi @leverage-analytics , I've pushed a fix for this. Does this issue still occur after syncing with main?
@adhtruong thanks for the info. I'll sync the changes and let you know if I'm still having any issues.