Enable static type checks using mypy in CI#601
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #601 +/- ##
=======================================
Coverage 98.94% 98.94%
=======================================
Files 6 6
Lines 757 757
=======================================
Hits 749 749
Misses 8 8 ☔ View full report in Codecov by Sentry. |
|
In the summary, I wrote:
In our own Django application, it turns out only |
|
Thans for your throughout description of this PR. Is the |
I used it in several places in earlier versions of the annotations, but in the latest version there are no uses left. I'll drop it from this PR and we can see whether or not to add it later if/when there is a concrete use case to discuss. |
This does not require annotations yet, but it will check all code outside of functions.
Suppress rather than annotate, because people type checking against `django-model-utils` will always have it installed and therefore should not have to deal with `__version__` being `None`.
Django can handle both strings and integers, but typeshed expects the default value to match the mapping's value type.
Avoid using `Self` as a type argument: for some reason this fails when mypy has an empty cache, but passes when the cache has been filled. Maybe it's a weird interaction between the mypy core and the django-stubs plugin?
This is a first step in adding type annotations. See #558 for discussion.
Some things to pay attention to during review:
I'm using
from __future__ import annotationsto force postponed evaluation of annotations. This has several advantages:dict[str, str]orint | Noneto be used in Python versions where the interpreter doesn't support that syntax yetQuerySetin Django 3.2, whereQuerySetdoes not yet inherit fromtyping.GenericI added
typing_extensionsas a runtime dependency. This makes it possible to use both runtime and static features from newer versions of Python in older versions as well. In particular,typing.Selfsimplifies a lot of annotations, but was only added to the standard library in Python 3.11.It is possible to only use static features of
typing_extensionsand put the import within aif TYPE_CHECKING:guard. It makes maintenance a bit more cumbersome, but it would eliminate the runtime dependency. As a lot of other libraries are usingtyping_extensionsalready, I'm not sure if it avoiding the dependency indjango-model-utilswould actually make a difference in any real-world application. But if you want me to make that change, let me know.I've used Python 3.8 in the tox configuration to run mypy, because that is the one that also runs flake8 and isort. I'm not sure that's a good reason though. Another option would be to use Python 3.12, as that is the fastest. Note that mypy's results do not depend on the Python version it runs under: it always has access to all typing features it supports.
The type checking of the unit tests requires all imported libraries to exist, which is why I added
time_machineas a dependency of the mypy env in tox. If we want to avoid that duplication, maybe it would make sense to create arequirements-testlib.txtor add atestlibextra tosetup.py.It would also be possible to move
time_machineintorequirements-test.txt, but installing the test tools and runtime dependencies likepsycopg2-binaryisn't necessary for mypy to work and I wanted to avoid slowing down CI.