-
Notifications
You must be signed in to change notification settings - Fork 397
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
feat: Add mypy type checking #4863
base: main
Are you sure you want to change the base?
Changes from 8 commits
4eb8e56
e92c2ef
1b553f0
83aebeb
8abf743
359317e
319a14e
a9fa1cd
a410134
b7c4974
1fa8173
91e79f3
dbe12f9
c2a9cf4
d63f5c1
77b9e41
7847a51
2281d5b
45b6f95
d572a36
2e79ae8
003c067
57f674d
57bab41
8895a11
5ca6332
408e9ad
c9b44d4
20388f8
fb7de2b
d0f3931
1393a65
96cf304
63b34a6
b2d7a4a
6297bce
7a36638
55165c9
fe37a4d
0de9e73
a382ad6
6f3b21d
21955b8
2bba3ae
522cd57
0d8a0d5
5c8449f
ac0b7cc
158ac2b
e2ace23
6d17eb0
c3e7907
fd7574c
264ecb7
6dcff38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
matthewelwell marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[mypy] | ||
exclude = ^(tests/)$ | ||
plugins = mypy_django_plugin.main | ||
|
||
[mypy.plugins.django-stubs] | ||
django_settings_module = "app.settings.local" | ||
|
||
# Enable Django Rest Framework stubs. | ||
[mypy.plugins.drf-stubs] | ||
enabled = True |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import re | ||
import subprocess | ||
import sys | ||
from pathlib import Path | ||
|
||
|
||
def main(): | ||
baseline_path = Path("scripts/mypy_baseline.txt") | ||
if not baseline_path.exists(): | ||
print("Baseline file not found. Run mypy and create mypy_baseline.txt first.") | ||
sys.exit(1) | ||
|
||
# Run mypy and capture output | ||
result = subprocess.run( | ||
["poetry", "run", "mypy", "--config-file", "mypy.ini", "."], | ||
capture_output=True, | ||
text=True, | ||
) | ||
current_output = result.stdout.strip().splitlines() | ||
if result.returncode != 1: | ||
print("Error running mypy:", result.stderr) | ||
sys.exit(1) | ||
|
||
with open(baseline_path, "r") as f: | ||
baseline = set(line.strip() for line in f if line.strip()) | ||
|
||
# Detect new errors and remove information line | ||
current_errors = set(current_output) | ||
new_errors = current_errors - baseline | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea. I've implemented this with a helpful error message. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just realized that this doesn't actually work since |
||
pattern = r"Found (\d+) errors in (\d+) files \(checked (\d+) source files\)" | ||
removal = None | ||
|
||
for error in new_errors: | ||
has_match = re.match(pattern, error) | ||
if has_match: | ||
removal = error | ||
break | ||
|
||
if removal: | ||
new_errors.remove(removal) | ||
|
||
if new_errors: | ||
print("New mypy errors detected:") | ||
print("\n".join(new_errors)) | ||
sys.exit(1) | ||
|
||
print("No new mypy errors detected.") | ||
sys.exit(0) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
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.
Since I was looking at similar functionality yesterday (see here) , I am somewhat intrigued by this.
My understanding of pre-commit is that it passes the modified files into the command specified in
entry
, so the resulting command would look something like:... which I would expect to just not work at all, or at least throw some sort of error?
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.
@zachaysan I'm not sure the latest changes make any reference to this?
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.
I've altered the script to run the files one at a time. I had to use
git diff
to list the target files of the commit because no matter what I tried to pass into the bash script it just wouldn't run with the automatic listed files. The good news is that we are now doing amypy
check on only the committed files.