-
Notifications
You must be signed in to change notification settings - Fork 311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Nox session that uses autotyping
to automatically add type hint annotations
#2696
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2696 +/- ##
=======================================
Coverage 94.36% 94.36%
=======================================
Files 107 107
Lines 9458 9458
Branches 2184 2184
=======================================
Hits 8925 8925
Misses 345 345
Partials 188 188 ☔ View full report in Codecov by Sentry. |
@@ -203,10 +203,25 @@ def linkcheck(session: nox.Session): | |||
session.run(*sphinx_commands, *check_hyperlinks, *session.posargs) | |||
|
|||
|
|||
MYPY_TROUBLESHOOTING = """ | |||
🦺 To learn more about type hints, check out mypy's cheat sheet at: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fortunately this particular emoji shows up nicely in the output of GitHub Actions! I expect that it'll be useful to draw people's attention to the troubleshooting information.
@@ -542,9 +542,6 @@ def create_tracker_obj(**kwargs) -> cpr.Tracker: | |||
return sim | |||
|
|||
|
|||
tracker_obj_simulated = create_tracker_obj(field_weighting="nearest neighbor").run() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the changes from autotyping
, mypy
found that the run
method here was returning None
, which ended up being the value passed to tracker_obj_simulated
. This was one example of how static type checking can be helpful!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! Type hints are underutilized in Python IMO, so I support anything like this that makes them easier to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🏕️ Alphabetized the lists within this file primarily to trigger a new RTD build since it had stalled.
This PR adds the
autotyping
session tonoxfile.py
✨, along with some mypy troubleshooting information that gets printed out when mypy runs.This session provides two autotyping options so far:
safe
: which will add type hints that have a very high probability of being correctaggressive
: which will add type hints that will usually be correct but sometimes may not be. Since using this option will necessitate a careful code review, it should never be used in CI.📌 In the not-too-distant future, I'd like to either add this to our CI, or provide better instructions for contributors on how to use this. Alternatively, we could run this with the
safe
option as a pre-commit hook, which would let contributors add type hints to PRs withpre-commit.ci autofix
comments. I'm hoping this will make adding type hints easier.🌠 We could also add rules where if a variable is named
B
, then a type hint ofu.Quantity[u.B]
gets automatically added, but I'll save that for a standalone PR.📝 A limitation of
autotyping
is that it will only add simple type hints rather than more complicated ones. We could potentially also add a Nox session for other tools like monkeytype which will run pytest and then apply type hints based on the type hints found at runtime. We wouldn't be able to include that in CI because changes from those tools will require careful code review.