Now that we’ve deprecated Python 2 support, I’m wondering if anyone has thought about adding type hints to numba. If the infrastructure to run mypy was in place, this could be a good thing for community contributions.
I agree. From gitter ealier this year
luk-f-a @luk-f-a Apr 14 19:28
style question: can we add type annotations in internals when we’re developing new features? (at the cost of inconsistent coverage?)
I find them extremely useful, and I wouldn’t mind adding them to the code I touch. But I would understand if you don’t like having little islands of annotations here and there.
Siu Kwan Lam @sklam Apr 14 20:17
if we are adding type annotations, we should check that they are correct as part of public CI
I don’t mind type annotations. I can see that it’s useful in some cases.
So it seems that devs are ok with it, but someone needs to set up CI. I don’t know how to do it unfortunately.
I think they would help a LOT in community contributions.
Thanks for sharing the thread from gitter. I completely agree that internal type annotations would be very helpful for community contributors (selfishly that’s part of why I asked about it ;). I agree with @sklam that we should make sure type annotations are correct.
If we want to do an incremental rollout (i.e. people add them to a few files at a time), we would probably want the ability to configure a list of files to validate with mypy, possibly with follow_imports = "silent" for now. My impression is that modifying the CI build requires some core developer work. I’d like to hear what the core team thinks about this.
I think this could be done by a contributor if they were to enable Azure checks on their own repository. I’ve done that before, it’s not difficult. It’d take adding a new script to https://github.com/numba/numba/blob/master/azure-pipelines.yml and then probably duplicating https://github.com/numba/numba/blob/92df8df57dd31a3da51d3b0cf94a4ec2b591f71a/buildscripts/azure/azure-linux-macos.yml#L29-L34 to run mypy instead of flake8 in the new CI job.
Setting up CI on a forked repo wouldn’t ensure that another PR doesn’t break existing type hints, no?
I think Azure picks up the build scripts from the PR branch. You can submit a draft PR to test adding mypy.
Good news everyone! Setting the CI mypy run was extremely easy. It worked without problems on the first try.
I have opened PR #5960 with the ci pipeline change, initial mypy configuration, and a few annotations necessary to get some modules passing.
We can proceed incrementally. Anyone that feels like contributing can add one module/package to the files list in mypy.ini and add all necessary annotations until mypy is happy.
In the ini file I have defined 4 levels of files, from strict to loose. Level 4 is no checking at all. Level 3 is basic compliance (many variables will be inferred to by Any, functions/variables are annotated only to prevent errors but not complete). Level 2 requires all functions to be annotated. Level 1 is stricter.
Any feedback is welcome, either here or in the PR. Please consider that it’s my first time setting mypy (I do use it daily at work) so I’m sure there’s plenty of room for improvement. I bet there can be heated debates about how strict it makes sense to make level 1. I defined it as an aspiration, but if it doesn’t work in practice, it can be relaxed.
happy typing!
Luk
2 Likes