Repository navigation
Add mypy to continuous integration suite - #2432
Conversation
github-actions
Bot
added
CI
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2432 +/- ##
=======================================
Coverage 96.92% 96.92%
=======================================
Files 104 104
Lines 9153 9153
=======================================
Hits 8872 8872
Misses 281 281 ☔ View full report in Codecov by Sentry. |
| @@ -158,4 +158,4 @@ commands = python -c 'import plasmapy' | |||
| deps = | |||
| mypy >= 1.8.0 | |||
There was a problem hiding this comment.
I'm wondering if there's an alternative to having to update this manually... dependabot, maybe?
| mypy >= 1.8.0 | ||
| commands = | ||
| mypy plasmapy --install-types --non-interactive | ||
| mypy plasmapy --install-types --non-interactive --show-error-context --show-error-code-links --pretty |
There was a problem hiding this comment.
We'll probably want to play with these flags to figure out which error output works best.
|
It's still really surprising to see mypy passing in our CI suite! I mean...we've got it set to ignore ∼700 errors, but still! 😺 |
StanczakDominik
left a comment
StanczakDominik
left a comment
There was a problem hiding this comment.
Looks quite sensible, let's try it out :) Please allow me to do something I haven't done in a while...
|
Thank you for the reviews! I'm curious about what @rocco8773 thinks about this too. I'm really happy to have this added to our CI suite since it's been something I've been hoping to do for several years. Enabling mypy will level up the quality of our code and documentation, but my main worry is that it'll be an extra barrier to entry for new contributors. So, we should keep thinking of our mypy configuration as experimental. I currently have it set so that new modules are expected to follow mypy's strict settings, but we may want to loosen those settings later on for some modules. @StanczakDominik — It's super awesome to hear from you and I hope you've been doing really well! |
Description
This PR adds
mypyto our continuous integration suite.Motivation and context
mypy is the de facto static type checker in the pythoniverse. It analyzes source code directly and makes sure that type hint annotations are used consistently throughout the package.
Adopting mypy in our CI will help us:
Additional considerations or tradeoffs are:
astropy.unitsastropy/astropy#15808).astropy.units. We can also add type stub files (like in Add type stub file for@wrapt.decorator#2442) for important functionality, or# type: ignore[error-code]comments for less important situations. These would need only include the functionality that we actually use.Related issues
Closes #268, which was raised almost six years ago!
In #2424, we added a mypy configuration file that ignores existing errors on a per-file and per-error basis. In #2431, we added a tox environment for running mypy. This PR adds that tox environment to our suite of continuous integration checks.
We also changed
@validate_quantitiesto work withu.Quantity[u.m]style comments in #2346, following by actual changes to the annotations in #2356 and #2421.