feat: Add basic typing support

[last-partizan]

This PR adds basic type hinting support

Refs #468

[jeffwidman]

Thanks!

[rbarrois]

That's interesting, thanks!

I've got a few questions though:

• How could we ensure that the annotations are actually working and useful? Can we test those?
• There doesn't seem to be a simple way to link the Generic[T] with the class Meta: model

[last-partizan]

@rbarrois

hmm, i think, to properly test type annotations, we should add pyright and/or mypy to CI, run it over example file, and compare that to expected output.

But there is simpler option, we can do something like this:

def x() -> int:
    ...

assert x.__annotations__ == {"returns": int}

In theory, this should be enough to ensure we get expected annotations.

as for linking with Meta: model, yes, i don't see any way to do it.

Also, i see some failing CI checks, i'll try to fix it.

[last-partizan]

Oh, looks like all errors is from 3.6 version, lefetime of which is ended three weeks ago.

Maybe better solution in this case, is to remove old version from CI? But i'll try to find other solutions, maybe any ideas?

[last-partizan]

I haven't found easy workarounds for this, so i created PR for removing 3.6 from CI, and adding 3.10 and django-4.0.

[mathieutu]

Thanks, an other huge step would be to type the kwargs.
To be able to do UserFactory.create(xxx=yy) and have autocompletion on xxx.

I have no idea on how to do that, and even if it could be possible with python? 🤔

[last-partizan]

I think, it could be implemented. Python 3.10 added ParamSpec, and it could be used with older versions of python via typing-extensions, so i think it could be possible.

[last-partizan]

I found a way to properly type even calls to factory, turned out it was pretty simple.

Now types are correct with both possible ways to call a factory:

BookFactory() -> Book
BookFactory.create() -> Book

But, this all requires python 3.6 at least.

[mathieutu]

I think, it could be implemented. Python 3.10 added ParamSpec, and it could be used with older versions of python via typing-extensions, so i think it could be possible.

I would love to see that!

[last-partizan]

I found a way to properly type even calls to factory, turned out it was pretty simple.

Turned out it doesn't work. Reverted changes back to only typing create, build, etc...

Now i'm trying to test it.

[last-partizan]

As for testing type annotations, i found one way to do it:

And it looks like this: https://github.com/smarie/python-decopatch/pull/27/files

• Install pyright
• Run it in json mode, compare results to snapshot

I could add similar tests for this package. (In this PR or next, preferrably next)

[last-partizan]

I fund myself today thinking it would be great to have types in my factories. Can we merge this, or we need to add tests first?

[youtux]

I started a project, types-factory-boy, to add type annotations to factory boy. It is in early stage, and it most probably needs a plugin to automatically discover and create type annotations automatically (otherwise you would have to type annotate basically every attribute), but I think it's a start.

[flaeppe]

When it comes to testing types and type annotations. I'd suggest: https://github.com/TypedDjango/pytest-mypy-plugins

Although it's a pytest plugin, so those tests needs to be run with pytest. Should be possible to add that as an individual job in CI etc.

Have a look at django-stubs https://github.com/typeddjango/django-stubs/tree/master/tests/typecheck for reference of how to use it and/or set it up.

[last-partizan]

@youtux great project, thank you.

After pyright fixed a bug with invalid init annotations and default values in 1.1.264, default factory_boy is throwing type errors sometimes (Maybe declaration for example).

Your project fixed it.

[jeffwidman]

Is this ready for review again?

[last-partizan]

In basic form, yes.

But, we have https://github.com/youtux/types-factory-boy now, and i can try to merge it into this project with merge_pyi.

But, i think it's better to leave this PR in minimal form, maybe add tests for types in the next PR. And then - merge types-factory-boy for more complete type coverage.

[youtux]

I think there was some misunderstanding here, as I don't understand @last-partizan message about merging types-factory-boy into this codebase.

types-factory-boy contains annotations for factory_boy lib (which I don't control), not for pytest_factoryboy (that I manage).

EDIT: scrap that, I thought I was writing in the pytest_factoryboy project.

[youtux]

I made some progress in types-factory-boy. Now there is a mypy plugin that automatically adds the typevar information of the Factory[T] baseclass.

So, instead of writing this:

class AuthorFactory(Factory[Author]):
    class Meta:
        model = Author
reveal_type(AuthorFactory.create())  # foo.Author

we can write this:

class AuthorFactory(Factory):  # No need to specify the typevar for Factory[...], it's inferred from Meta.model
    class Meta:
        model = Author
reveal_type(AuthorFactory.create())  # foo.Author

[palgorhythm]

Any updates on whether this PR will be merged?

[francoisfreitag]

I don’t see opposition to merging this work. I offered a small fixes as a PR. Once these fixes are correct, and integrated into this PR, I think we can merge it.

In the meantime, @rbarrois’ opinion is very welcome :)

[last-partizan]

Oh, I missed that PR. Fixes accepted!

[francoisfreitag]

Looks like a good start. Let’s hope type information PR build up to better document the library :)

I’ll leave the PR sit for a couple more weeks, to give time for others to report their concerns. In the absence of opposition, I’ll merge it early 2024.

[Viicos]

I think it doesn't hurt to type size as well

[francoisfreitag]

Can you rebase one last time before merging, please? 🙏

[last-partizan]

Sure, rebased already, but looks like typing tests aren't running in CI.

Almost ready.

[last-partizan]

Got it!

Ready to be merged, but please take another look at github workflow file and tox.ini

[rbarrois]

Thanks for all the hard work! (And sorry for my long absence :/)

I'm going to do a final pass, before merging this; I'm likely going to rebase or squash some of the commits, for easier further maintenance!

[rbarrois]

Thanks a lot for the work on this!

The basic typing annotation look fine, and are a great starting point; I'd like to merge them as quickly as possible.

I'm not too sure about the py.typed file though: it's unclear to me whether it means that the project is fully typed, or merely that it supports some typing annotations — PEP 561 is not very clear on that.

Regarding the testing setup, I have met a few issues running it locally; for now, I would rather merge the annotated code without those tests, or with some much simpler ones (e.g using mypy.reveal_types directly).

Suggested next steps:
@last-partizan if you're OK with this, I propose rebasing your branch to squash the various intermediate commits, and simplifying the test setup to reduce the amount of extra deps.
Would that sound right with you?

Details on the testing issues below:

• The make test-typing only works in a blank virtualenv without, e.g., django being installed. This is not the way we run tests usually: we use make test and others in a venv where make update was run to setup all dependencies.
• I would rather limit the testing dependencies to the stdlib if possible; depending on pytest would also confuse new contributors as to the way to run the test suite.

[rbarrois]

It would help to explain why this file is here, as it's not an actual "data" file.

Suggested change

	factory =
	# Mark the module as typed, see PEP 561
	factory =

[rbarrois]

Based on PEP 561, I'm not sure we can add this yet: the project is not fully typed.

[last-partizan]

@rbarrois i'm ok with whatever fits you.

If you want to simplify tests or remove py.typed - go ahead.

My main interest - is getting started with typing this package, and then improve types when i encounter problems in my projects.

[Viicos]

Regarding the testing setup, I have met a few issues running it locally; for now, I would rather merge the annotated code without those tests, or with some much simpler ones (e.g using mypy.reveal_types directly).

One alternative to pytest-mypy-plugins is to do it "manually": you create a python file that you want to test with mypy (or any type checker), and use typing_extensions.assert_type. You can assert for errors as well, by adding a type: ignore comment and making sure that mypy is configured to flag unused type: ignore comments as an error.

This solution does require some more work, but is imo more robust as you don't rely on how mypy errors messages are formatted (that could change from one version to another), and can be ported to another type checker if necessary (assert_type is standard Python).

Here is an example of how I do it (test files are under testfiles, and test_stubs.py is where the actual testing happens).

Based on PEP 561, I'm not sure we can add this yet: the project is not fully typed.

I'm not sure it should be included either. Without it, IDEs should be smart enough to still use the type definitions, but type checking with the command line will not.

[rbarrois]

Thanks again for all this work, I have merged it as #1059 !