Add mypy to CI #693
Add mypy to CI #693
Conversation
Use the lightweight config from
https://github.com/pyca/cryptography/blob/main/pyproject.toml#L24
for now.
placeholder for now, to get mypy passing
Sadly I can't import `SupportBytes` from typing_extensions, so for 3.8 and below I need to define it myself.
|
Not sure why the pypy CI build failed. Looks like it couldn't build |
Looks like there was a related issue on aiohttp, but I don't know the details: aio-libs/aiohttp#5496 |
Seems like the underlying cause is python/typed_ast#111 . One of the linked PRs was nolar/kopf#845 , which decided to only run mypy under CPython. So maybe it's best to pull |
|
I would prefer that this be structured like the mypy job on Other than that this looks good, although I'd also like to get the Thanks for all the work you're doing here; this will be a great addition for the next release. |
|
@reaperhulk Many thanks, I'll try to get that done soon. But to double check:
Were you thinking of pyca/bcrypt here? AFAICS the pyca/cryptography tox configuration lumps mypy in to the |
|
Thanks for working with me on this! I look forward to the next PR 😄 If at all possible it'd be great to split this into as many PRs as is reasonably feasible. |
Following on from pyca#693, I've pulled out the changes in pyca#692 which relate to `nacl.exceptions` (and applied a suggestion of @Dreamsorcerer in the process). I picked this module because it was a leaf: it doesn't import any other things from `nacl`. The code changes are small; instead, I think we'll have more to say about the mypy config changes. I've turned on a bunch of extra checks for `nacl.exceptions` only. I'd hoped that I could enable strict mode on a module-by-module basis, but that doesn't seem to be possible. So instead, I went through the [mypy documentation](https://mypy.readthedocs.io/en/stable/config_file.html) and turned on every check that sounded good for type safety. (At work, I've found disallowing the propagation of Any, and `disallow_untyped_defs` the most useful: they help ensure that mypy is actually able to do type checking in the first place!) If you're happy with this list of checks, my plan would be to take on other modules and add them one-by-one to the "strict" section in `pyproject.toml`. In the long run I'd like to have everything in `pynacl` under this list, at which point we can drop the per-module override and have a single set of package-wide configuration for mypy.
My pleasure: thank you for having me. My plan is to tackle each module in a PR by itself, starting with #694. |
Following on from #693, I've pulled out the changes in #692 which relate to `nacl.exceptions` (and applied a suggestion of @Dreamsorcerer in the process). I picked this module because it was a leaf: it doesn't import any other things from `nacl`. The code changes are small; instead, I think we'll have more to say about the mypy config changes. I've turned on a bunch of extra checks for `nacl.exceptions` only. I'd hoped that I could enable strict mode on a module-by-module basis, but that doesn't seem to be possible. So instead, I went through the [mypy documentation](https://mypy.readthedocs.io/en/stable/config_file.html) and turned on every check that sounded good for type safety. (At work, I've found disallowing the propagation of Any, and `disallow_untyped_defs` the most useful: they help ensure that mypy is actually able to do type checking in the first place!) If you're happy with this list of checks, my plan would be to take on other modules and add them one-by-one to the "strict" section in `pyproject.toml`. In the long run I'd like to have everything in `pynacl` under this list, at which point we can drop the per-module override and have a single set of package-wide configuration for mypy.
Pulled out from pyca#692. After we realised in pyca#693 that we don't need `typing_extensions` to use `SupportsBytes`, I didn't want to pull in `typing_extensions` again for just one use of `Protocol`. (I have no other plans to use `Protocol` in other annotations.) My understanding: doing so means that we can't run `mypy` under Python 3.6 or 3.7 to fully typecheck the package. Maybe that's fine? I guess the downside is that someone using `nacl` in a 3.6 or 3.7 project can't fully benefit from these annotations. What're your thoughts here? (As a compromise, I could do a `try-except` dance to use from `typing_extensions.Protocol`, if it happens to be available.)
* Type annotations for `nacl.encoding` Pulled out from #692. After we realised in #693 that we don't need `typing_extensions` to use `SupportsBytes`, I didn't want to pull in `typing_extensions` again for just one use of `Protocol`. (I have no other plans to use `Protocol` in other annotations.) My understanding: doing so means that we can't run `mypy` under Python 3.6 or 3.7 to fully typecheck the package. Maybe that's fine? I guess the downside is that someone using `nacl` in a 3.6 or 3.7 project can't fully benefit from these annotations. What're your thoughts here? (As a compromise, I could do a `try-except` dance to use from `typing_extensions.Protocol`, if it happens to be available.) * Use an ABC instead of a protocol * Format with black * Docstrings to appease the `coverage` check
Following the strategy outlined in #692 (comment), I've