Skip to content
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

Open
wants to merge 55 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
4eb8e56
Add mypy type checking to pre-commit
zachaysan Nov 25, 2024
e92c2ef
Add mypy to pyproject and update necessary dependencies
zachaysan Nov 25, 2024
1b553f0
Create mypy.ini
zachaysan Nov 25, 2024
83aebeb
Add __init__.py files for mypy
zachaysan Nov 25, 2024
8abf743
Fix failing lint
zachaysan Nov 25, 2024
359317e
Add mypy baseline failures
zachaysan Nov 25, 2024
319a14e
Add mypy checking script
zachaysan Nov 25, 2024
a9fa1cd
Move mypy to the end since it is the slowest
zachaysan Nov 25, 2024
a410134
Delete mypy.ini
zachaysan Dec 3, 2024
b7c4974
Add mypy concerns to pyproject.toml
zachaysan Dec 3, 2024
1fa8173
Update mypy checking
zachaysan Dec 3, 2024
91e79f3
Add type ignores
zachaysan Dec 3, 2024
dbe12f9
Fix conflicts and merge branch 'main' into feat/add_mypy_type_checking
zachaysan Dec 3, 2024
c2a9cf4
Attempt passing args into mypy_check.py script
zachaysan Dec 4, 2024
d63f5c1
Attempt using $FILES
zachaysan Dec 4, 2024
77b9e41
Debug command line args
zachaysan Dec 4, 2024
7847a51
Attempt using git for the filenames
zachaysan Dec 4, 2024
2281d5b
Update mypy check with success exit code and other changes
zachaysan Dec 4, 2024
45b6f95
Fix conflicts and merge branch 'main' into feat/add_mypy_type_checking
zachaysan Dec 4, 2024
d572a36
Update mypy_baseline
zachaysan Dec 4, 2024
2e79ae8
Filter out mypy failures for site packages
zachaysan Dec 4, 2024
003c067
Add errors to baseline
zachaysan Dec 4, 2024
57f674d
Update baseline and revert changes to util model type ignores
zachaysan Dec 4, 2024
57bab41
Remove command from script output
zachaysan Dec 5, 2024
8895a11
Add baseline updating to mypy check script
zachaysan Dec 9, 2024
5ca6332
Fix conflicts and merge branch 'main' into feat/add_mypy_type_checking
zachaysan Dec 9, 2024
408e9ad
Try setting up mypy check for CI
zachaysan Dec 9, 2024
c9b44d4
Revert now acceptable check since mypy doesn't generate errors for ev…
zachaysan Dec 9, 2024
20388f8
Add mypy type checking to pre-commit
zachaysan Nov 25, 2024
fb7de2b
Add mypy to pyproject and update necessary dependencies
zachaysan Nov 25, 2024
d0f3931
Create mypy.ini
zachaysan Nov 25, 2024
1393a65
Add __init__.py files for mypy
zachaysan Nov 25, 2024
96cf304
Add mypy baseline failures
zachaysan Nov 25, 2024
63b34a6
Add mypy checking script
zachaysan Nov 25, 2024
b2d7a4a
Move mypy to the end since it is the slowest
zachaysan Nov 25, 2024
6297bce
Delete mypy.ini
zachaysan Dec 3, 2024
7a36638
Add mypy concerns to pyproject.toml
zachaysan Dec 3, 2024
55165c9
Update mypy checking
zachaysan Dec 3, 2024
fe37a4d
Add type ignores
zachaysan Dec 3, 2024
0de9e73
Attempt passing args into mypy_check.py script
zachaysan Dec 4, 2024
a382ad6
Attempt using $FILES
zachaysan Dec 4, 2024
6f3b21d
Debug command line args
zachaysan Dec 4, 2024
21955b8
Attempt using git for the filenames
zachaysan Dec 4, 2024
2bba3ae
Update mypy check with success exit code and other changes
zachaysan Dec 4, 2024
522cd57
Update mypy_baseline
zachaysan Dec 4, 2024
0d8a0d5
Filter out mypy failures for site packages
zachaysan Dec 4, 2024
5c8449f
Add errors to baseline
zachaysan Dec 4, 2024
ac0b7cc
Update baseline and revert changes to util model type ignores
zachaysan Dec 4, 2024
158ac2b
Remove command from script output
zachaysan Dec 5, 2024
e2ace23
Add baseline updating to mypy check script
zachaysan Dec 9, 2024
6d17eb0
Try setting up mypy check for CI
zachaysan Dec 9, 2024
c3e7907
Revert now acceptable check since mypy doesn't generate errors for ev…
zachaysan Dec 9, 2024
fd7574c
Fix conflicts and merge branch 'feat/add_mypy_type_checking' of githu…
zachaysan Dec 17, 2024
264ecb7
update baseline
khvn26 Dec 17, 2024
6dcff38
sort script output
khvn26 Dec 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,13 @@ repos:
- id: poetry-check
args: ['-C', 'api']

- repo: local
hooks:
- id: mypy-check
name: mypy check
entry: bash -c "cd api && poetry run python scripts/mypy_check.py"
Copy link
Contributor

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:

bash -c "cd api && poetry run python scripts/mypy_check.py" file1.py file2.py 

... which I would expect to just not work at all, or at least throw some sort of error?

Copy link
Contributor

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?

Copy link
Contributor Author

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 a mypy check on only the committed files.

language: system
types: [python]

ci:
autoupdate_commit_msg: 'ci: pre-commit autoupdate'
4 changes: 3 additions & 1 deletion api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,9 @@

SECURE_REDIRECT_EXEMPT = env.list("DJANGO_SECURE_REDIRECT_EXEMPT", default=[])
SECURE_REFERRER_POLICY = env.str("DJANGO_SECURE_REFERRER_POLICY", default="same-origin")
SECURE_CROSS_ORIGIN_OPENER_POLICY = env.str("DJANGO_SECURE_CROSS_ORIGIN_OPENER_POLICY", default="same-origin")
SECURE_CROSS_ORIGIN_OPENER_POLICY = env.str(
"DJANGO_SECURE_CROSS_ORIGIN_OPENER_POLICY", default="same-origin"
)
SECURE_SSL_HOST = env.str("DJANGO_SECURE_SSL_HOST", default=None)
SECURE_SSL_REDIRECT = env.bool("DJANGO_SECURE_SSL_REDIRECT", default=False)

Expand Down
Empty file.
Empty file.
10 changes: 10 additions & 0 deletions api/mypy.ini
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
812 changes: 714 additions & 98 deletions api/poetry.lock

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ django-ordered-model = "~3.4.1"
django-ses = "~3.5.0"
django-axes = "~5.32.0"
pydantic = "^2.3.0"
pyngo = "~2.0.1"
pyngo = "~2.2.1"
flagsmith = "^3.6.0"
python-gnupg = "^0.5.1"
django-redis = "^5.4.0"
Expand Down Expand Up @@ -219,6 +219,10 @@ requests-mock = "^1.11.0"
django-extensions = "^3.2.3"
pdbpp = "^0.10.3"
mypy-boto3-dynamodb = "^1.33.0"
mypy = "^1.11.2"
django-stubs = "~5.1.1"
djangorestframework-stubs = "~3.14.0"
boto3-stubs = "~1.35.67"

[build-system]
requires = ["poetry-core>=1.5.0"]
Expand Down
1,532 changes: 1,532 additions & 0 deletions api/scripts/mypy_baseline.txt

Large diffs are not rendered by default.

52 changes: 52 additions & 0 deletions api/scripts/mypy_check.py
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should check for baseline - current_errors as well to make sure the baseline file contains up to date errors. Rarely, but things may change implicitly (e.g. with a dependency update).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. I've implemented this with a helpful error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that this doesn't actually work since mypy only checks errors for the current working module. I've reverted my change.

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()
Loading