factory_boy icon indicating copy to clipboard operation
factory_boy copied to clipboard

Problems with the new type annotations

Open llazzaro opened this issue 10 months ago • 17 comments

Hello,

First of all, I really appreciate the improvements in version 3.3.3, especially the addition of type annotations—it’s a great step forward! However, I wanted to share some feedback on the impact of this change.

Since the update, it’s breaking pipelines in several projects that I maintain using Factory Boy. We now need to update all our test code to include type annotations or ignore modules. Given the scope of the change, it might have been better suited for a major version update rather than a minor one.

I just wanted to provide some visibility into the challenges we’re facing so it can be considered for future updates.

Thanks for all your work on this!

llazzaro avatar Feb 04 '25 15:02 llazzaro

Thanks for the feedback! I'm really sorry about that :/ In my mind, the simple addition of the types a few releases back would have made those annotations available to all downstream consumers, and not including those was an oversight.

Is there a way I can add some stuff to adjust the type annotations, that wouldn't break your CI?

rbarrois avatar Feb 05 '25 08:02 rbarrois

I had some problems too. I started using the generics feature as the initial support was added and it worked fine. With the adition of the py.typed file in 3.3.3 (I think) it seems like mypy started to actually use the types... In particular, we're getting an error in a post-generation method:

error: "UserFactory" has no attribute "set_password"; maybe "password"?

The code lives here: https://github.com/cookiecutter/cookiecutter-django/blob/master/%7B%7Bcookiecutter.project_slug%7D%7D/%7B%7Bcookiecutter.project_slug%7D%7D/users/tests/factories.py but it's a cookiecutter template, which renders to (more or less) that in generated project:

class UserFactory(DjangoModelFactory[User]):
    username = Faker("user_name")
    email = Faker("email")
    name = Faker("name")

    @post_generation
    def password(self, create: bool, extracted: Sequence[Any], **kwargs):  # noqa: FBT001
        password = (
            extracted
            if extracted
            else Faker(
                "password",
                length=42,
                special_chars=True,
                digits=True,
                upper_case=True,
                lower_case=True,
            ).evaluate(None, None, extra={"locale": None})
        )
        self.set_password(password)  # error

    class Meta:
        model = User
        django_get_or_create = ["username"]

The set_password method is from Django's user model.

browniebroke avatar Feb 06 '25 08:02 browniebroke

@rbarrois

Is there a way I can add some stuff to adjust the type annotations, that wouldn't break your CI?

https://github.com/FactoryBoy/factory_boy/pull/1114 will allow us to upgrade, otherwise we get hunderds of import errors

kalekseev avatar Feb 10 '25 10:02 kalekseev

I can also confirm that #1114 is the way to go, which explicitly states what the public interface is. Otherwise, it breaks all pyright type checking, as implicit private imports are not allowed with this type checker.

medihack avatar Feb 10 '25 17:02 medihack

Not sure if our setup is special, but with Django all type checks on *Factory instances are now failing with errors like:

 error: "UserFactory" has no attribute "pk"  [attr-defined]
 error: "UserFactory" has no attribute "refresh_from_db"  [attr-defined]

with:

class UserFactory(DjangoModelFactory):
    class Meta:
        model = User
     
    username = factory.Faker("email")
    ...

gersmann avatar Feb 11 '25 08:02 gersmann

@gersmann

class UserFactory(DjangoModelFactory[User]):
    class Meta:
        model = User
     
    username = factory.Faker("email")
    ...


user = UserFactory.create(...)  # user have type User

kalekseev avatar Feb 11 '25 08:02 kalekseev

@kalekseev , we can use

[mypy-factory]
implicit_reexport = True

waiting for #1114

Dose Factory.create() instead of Factory() is the recommended way to avoid typing error (we have a bit more than 1000 calls in our test-suite, so i prefer to be sure i am not going back in a few weeks

yobuntu avatar Feb 12 '25 15:02 yobuntu

Dose Factory.create() instead of Factory() is the recommended way to avoid typing error

@yobuntu AFAIK it's the only way to get underlying model, Factory() returns object of type Factory.

kalekseev avatar Feb 12 '25 15:02 kalekseev

@kalekseev , thanks,

Is there a way to provide the same typing for SubFactory (and factory.Sequence) as in this exemple:

class DepartmentFactory(BaseFactory):

    class Meta:
        model = Department

    id = factory.Sequence(lambda n: f"{n}")
    old_region = factory.SubFactory(RegionFactory)

would throw the same kind of untyped var in typed context

yobuntu avatar Feb 12 '25 16:02 yobuntu

@yobuntu

Is there a way to provide the same typing for SubFactory

if you create dep = DpartmentFactory.create() then dep should be of type Department and dep.id will be of type Department.id so it doesn't matter how you defined these fields inside factory

kalekseev avatar Feb 12 '25 19:02 kalekseev

I totally agree at runtime, but when doing static type checking, mypy find this error:

error: Call to untyped function "SubFactory" in typed context [no-untyped-call]

edit: I was not able to find a proper fix, but as in all tests, i only use DepartmentFactory create method. it is not so bad to use the pragma type: ignore on the line defining the SubFactory

yobuntu avatar Feb 13 '25 08:02 yobuntu

@yobuntu

error: Call to untyped function "SubFactory" in typed context [no-untyped-call]

factory boy is missing a lot of types currently, if you store factories in separate files you can disable that rule with

[[tool.mypy.overrides]]
module = [
  "yourproject.*.factories",
]
disallow_untyped_calls = false

@browniebroke

In particular, we're getting an error in a post-generation method:

AFAIK there's no way to fix this but you can make def password(self: User, ...) # type: ignore and have self as User inside the function

Another way is to use factory.PostGeneration instead of decorator

kalekseev avatar Feb 17 '25 09:02 kalekseev

I totally agree at runtime, but when doing static type checking, mypy find this error:

error: Call to untyped function "SubFactory" in typed context [no-untyped-call]

edit: I was not able to find a proper fix, but as in all tests, i only use DepartmentFactory create method. it is not so bad to use the pragma type: ignore on the line defining the SubFactory

I have the same issue. Anything inside the factory itself (Faker, lazy_attribute, LazyFunction, SubFactory etc.) is untyped and causing mypy errors like these:

myproject/users/factories.py:28: error: Call to untyped function "Faker" in typed context  [no-untyped-call]
myproject/users/factories.py:28: note: See https://mypy.rtfd.io/en/stable/_refs.html#code-no-untyped-call for more info
myproject/users/factories.py:30: error: Call to untyped function "LazyFunction" in typed context  [no-untyped-call]
myproject/users/factories.py:31: error: Call to untyped function "lazy_attribute" in typed context  [no-untyped-call]

I am actually unsure whether there is a solution for these from a typing perspective since it depends on the usage. For now I added this to my mypy config:

[mypy-*.factories]
disallow_untyped_calls = False

mschoettle avatar Feb 17 '25 11:02 mschoettle

I agree with this comment that explicitly exporting the factory components/classes in the public interface will solve a large bulk of the (false-)positives. This PR seems to take care of it.

alichaudry avatar Feb 27 '25 22:02 alichaudry

We are not upgrading to latest factory-boy because of this issue. Could we please either fix it or comment?

sshishov avatar Jul 25 '25 08:07 sshishov

@browniebroke , I guess to properly support post_generation, we have to add the proper type hints for this decorator, then it will know that the function will be called without self and the first attribute should be annotated...

Am I missing something?

sshishov avatar Aug 03 '25 14:08 sshishov

Hi, just wanted to add to this:

We need to define __all__ in __init__.py otherwise using factory.Faker results in

error: Module "factory" does not explicitly export attribute "Faker" [attr-defined]

realsuayip avatar Nov 27 '25 12:11 realsuayip