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

Remove second mypy run from pre-commit #7344

Merged
merged 1 commit into from
Nov 24, 2022

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Nov 23, 2022

We introduced a second mypy check that runs in a platform compat mode for windows in #7094

Executing a single mypy check can take a significant amount of time. Below you can see the timings for me when running all hooks. As you can see, mypy takes about 15s on my machine. Adding an additional, mostly redundant 15s to every commit is not justified.
It's also worth pointing out that we're not running this for every python version even though mypy potentially is having the same problem there, see also python/mypy#10747

main ✔                                                                                                                                                                                        1d1h
▶ pre-commit run -av
absolufy-imports.........................................................Passed
- hook id: absolufy-imports
- duration: 0.3s
isort....................................................................Passed
- hook id: isort
- duration: 0.92s
pyupgrade................................................................Passed
- hook id: pyupgrade
- duration: 1.18s
black....................................................................Passed
- hook id: black
- duration: 0.17s

All done! ✨ 🍰 ✨
286 files left unchanged.

flake8...................................................................Passed
- hook id: flake8
- duration: 11.41s
codespell................................................................Passed
- hook id: codespell
- duration: 0.16s
mypy.....................................................................Passed
- hook id: mypy
- duration: 14.88s

Success: no issues found in 287 source files

mypy.....................................................................Passed
- hook id: mypy
- duration: 14.41s

Success: no issues found in 287 source files

cc @crusaderky you added this. Any strong objections?

@github-actions
Copy link
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       15 files  ±0         15 suites  ±0   6h 39m 4s ⏱️ - 2m 23s
  3 225 tests ±0    3 137 ✔️  - 1    83 💤  - 1  5 +2 
23 845 runs  ±0  22 931 ✔️  - 2  908 💤  - 1  6 +3 

For more details on these failures, see this check.

Results for commit 44e970e. ± Comparison against base commit 2963bf1.

@crusaderky
Copy link
Collaborator

Your flake8 runtime is abnormal.
My timings on linux:

flake8...................................................................Passed
- hook id: flake8
- duration: 1.97s
codespell................................................................Passed
- hook id: codespell
- duration: 0.11s
mypy.....................................................................Passed
- hook id: mypy
- duration: 21.14s

Not sure if it's because you're running on MacOSX but I'd be surprised if that was the case.

@crusaderky crusaderky merged commit 7f67c2c into dask:main Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants