Add mypy type checking#269
Merged
sethmlarson merged 1 commit intopython-distro:masterfrom Jul 1, 2021
jdufresne:mypy
Merged
Conversation
funkyfuture
reviewed
Jun 30, 2021
Contributor
funkyfuture
left a comment
funkyfuture
left a comment
There was a problem hiding this comment.
i find these changes very helpful and well done.
distro.py
Outdated
| import argparse | ||
| import subprocess | ||
|
|
||
| if False: |
Contributor
There was a problem hiding this comment.
what's the rationale for this condition? is that to work with Python 2, where no typing.TYPE_CHECKING is available? if so, a short comment would help in the long term for other readers.
amending # pragma: nocover to this line should calm down that automated commenter here.
Member
Author
There was a problem hiding this comment.
Thanks for the review @funkyfuture. I have applied your suggestions to the latest revision.
sethmlarson
suggested changes
Jul 1, 2021
Contributor
sethmlarson
left a comment
sethmlarson
left a comment
There was a problem hiding this comment.
The change looks good, just need to hook it up to GHA (and one more comment too)
Type checking helps build confidence that an internally consistent API is used correctly. Use type comments rather than annotations to retain compatibility with Python 2.7. The functools.cached_property is now used on Python 3.8+ as this helps mypy understand the type of the cached property. Rationale --------- The pip project vendors a copy of distro.py: https://github.com/pypa/pip/blob/20.3.3/src/pip/_vendor/distro.py pip also uses mypy. Right, pip must workaround or ignore type errors that occur from calls to distro.py. By including types directly in the library, it will help distro, pip, and any other library users running mypy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type checking helps build confidence that an internally consistent API
is used correctly.
Use type comments rather than annotations to retain compatibility with
Python 2.7.
The functools.cached_property is now used on Python 3.8+ as this helps
mypy understand the type of the cached property.
Rationale
The pip project vendors a copy of distro.py:
https://github.com/pypa/pip/blob/20.3.3/src/pip/_vendor/distro.py
pip also uses mypy. Right now, pip must workaround or ignore type errors
that occur from calls to distro.py. By including types directly in the
library, it will help distro, pip, and any other library users running
mypy.