Skip to content
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

Enhancement: have Trait/HiddenField only there to feed the post_generate of other fields #604

Open
MRigal opened this issue Nov 8, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@MRigal
Copy link

MRigal commented Nov 8, 2024

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


Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@MRigal MRigal added the enhancement New feature or request label Nov 8, 2024
@adhtruong
Copy link
Collaborator

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

@MRigal
Copy link
Author

MRigal commented Nov 13, 2024

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}"

@MRigal
Copy link
Author

MRigal commented Nov 13, 2024

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

@MRigal MRigal changed the title Enhancement: have HiddenField only there to feed the post_generate of others Enhancement: have Trait/HiddenField only there to feed the post_generate of other fields Nov 21, 2024
@MRigal
Copy link
Author

MRigal commented Nov 21, 2024

re-pinging @adhtruong as I'm interested to work out the feature if you give me some guidance :-)

@adhtruong
Copy link
Collaborator

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 of Param type and evaluate accordingly.
  • I think it would be nice to have this overridable as a kwarg passed in via build or process_kwarg. This allows some flexibility
  • Similar changes should also be made for process_kwargs_coverage to support usage that way.

Let me know if need anymore help to start or think should reduce scope of initial implementation! :)

CC: @guacs

@MRigal
Copy link
Author

MRigal commented Nov 26, 2024

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. :-)

@leverage-analytics
Copy link

leverage-analytics commented Dec 13, 2024

@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.

@leverage-analytics
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants