-
Notifications
You must be signed in to change notification settings - Fork 306
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2696 +/- ##
=======================================
Coverage 95.14% 95.14%
=======================================
Files 107 107
Lines 9411 9411
Branches 2168 2168
=======================================
Hits 8954 8954
Misses 276 276
Partials 181 181 ☔ 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!
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.