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

Add codespell to pre-commit config #18645

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Feb 9, 2025

Replaces #18387

Use codespell in pre-commit to detect spelling mistakes, see #18642. Ignore test and typeshed folders.

Copy link
Contributor

github-actions bot commented Feb 9, 2025

Diff from mypy_primer, showing the effect of this PR on open source code:

django-stubs (https://github.com/typeddjango/django-stubs): 2.04x faster (53.6s -> 26.3s in a single noisy sample)

@cdce8p cdce8p added topic-developer Issues relevant to mypy developers and removed topic-developer Issues relevant to mypy developers labels Feb 9, 2025
@JukkaL
Copy link
Collaborator

JukkaL commented Feb 10, 2025

How long does this take to run? Our current pre-commit config already feels slow when running locally. (Somewhat unrelated: I wonder if there is a way to move the slower tasks to only run CI by default?)

@cdce8p
Copy link
Collaborator Author

cdce8p commented Feb 10, 2025

How long does this take to run? Our current pre-commit config already feels slow when running locally.

A full run of just codespell (via pre-commit run codespell --all-files) takes 1.5s on my machine. Considering that all hooks in total take ~45s, I'd say it's probably negligible, especially as it's only run on changed files for a commit.

(Somewhat unrelated: I wonder if there is a way to move the slower tasks to only run CI by default?)

Yes, there is, although it doesn't fully work with the pre-commit.ci integration we currently use. We'd have to run these manually as part of an additional workflow job. The question would be which part is actually slow for you?

On my machine it's actionlint (with Lint GitHub Actions workflow files), however that's only run if workflow files change, so we can probably ignore it. The other "slow" one is black, however only the first full run. It's quite fast with the cache setup.

@sterliakov
Copy link
Collaborator

sterliakov commented Feb 10, 2025

(irrelevant to this PR)

The other "slow" one is black

and it's hella slow when it has to reformat the file, actually: 7-8 seconds for any edit of checker.py. Did we ever consider switching to ruff format that runs in sub-second time (at least 20x faster on both small and huge files, 0.08s for me on changed checker.py, 0.1-0.2 s to format whole ./mypy folder)? Or is black used as a cross-check for its mypyc-compiled wheels?

Black is probably the main reason why our pre-commit feels so slow locally when running on changed files - unless you edit GH actions, other hooks either are skipped (do not apply) or complete immediately.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 11, 2025

Yeah, we are using Black in part because it's compiled with mypyc, and it gives us an incentive to make mypyc better.

Though on my work laptop (M1 Max) reformatting checker.py takes about 1.5s. 7-8 seconds seems like a lot. Are you sure you are using a compiled version of Black?

Black should do much less work than mypy, but it appears to be slower than mypy when processing a single file (at least in some cases), so there may some potential performance wins to be had there.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Feb 11, 2025

Yeah, we are using Black in part because it's compiled with mypyc, and it gives us an incentive to make mypyc better.

I agree. Black has become a cornerstone of the Python ecosystem. If everybody just moves on to ruff, development of it would probably stall which would be unfortunate. I'd much rather see some more performance improvements, although that certainly depends on the contributors time.

For now, we should still with black.

--
What's the resolution for this PR? Are you comfortable with codespell @JukkaL or do you want me to explore other options like moving it to a CI only check? I think it's probably fast enough to be run as part of pre-commit itself, with <1.5s for a full run.

@sterliakov
Copy link
Collaborator

sterliakov commented Feb 11, 2025

@JukkaL hm, and that's interesting. Obviously run times vary from run to run, but...

$ wc -l mypy/checker.py && printf "'%s'" 'Some single quotes to fix' >>mypy/checker.py && git add . && time pre-commit run black ; git reset --hard
9092 mypy/checker.py
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted mypy/checker.py

All done! ✨ 🍰 ✨
1 file reformatted.


real    0m6.687s
user    0m6.565s
sys     0m0.133s
HEAD is now at f7f6bc206 Add initial changelog for 1.16 (#18652)

And yes, it seems to use a compiled version:

Pre-commit environment contents
$ find  ~/.cache/pre-commit/ -name black
/home/stas/.cache/pre-commit/repoqdziss5p/py_env-python3/bin/black
/home/stas/.cache/pre-commit/repoqdziss5p/py_env-python3/lib/python3.12/site-packages/black
$ ls /home/stas/.cache/pre-commit/repoqdziss5p/py_env-python3/lib/python3.12/site-packages/black -l
total 700
-rwxrwxr-x 1 stas stas  8432 Feb  1 21:03 brackets.cpython-312-x86_64-linux-gnu.so
-rw-rw-r-- 1 stas stas 12429 Feb  1 21:03 brackets.py                                                                                                                                                                                       
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 cache.cpython-312-x86_64-linux-gnu.so                                                                                                                                                             
-rw-rw-r-- 1 stas stas  4754 Feb  1 21:03 cache.py                                                                                                                                                                                          
-rwxrwxr-x 1 stas stas  8432 Feb  1 21:03 comments.cpython-312-x86_64-linux-gnu.so                                                                                                                                                          
-rw-rw-r-- 1 stas stas 15818 Feb  1 21:03 comments.py                                                                                                                                                                                       
-rw-rw-r-- 1 stas stas  6432 Feb  1 21:03 concurrency.py                                                                                                                                                                                    
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 const.cpython-312-x86_64-linux-gnu.so                                                                                                                                                             
-rw-rw-r-- 1 stas stas   321 Feb  1 21:03 const.py                                                                                                                                                                                          
-rw-rw-r-- 1 stas stas  1927 Feb  1 21:03 debug.py                                                                                                                                                                                          
-rw-rw-r-- 1 stas stas 14722 Feb  1 21:03 files.py                                                                                                                                                                                          
-rwxrwxr-x 1 stas stas  8456 Feb  1 21:03 handle_ipynb_magics.cpython-312-x86_64-linux-gnu.so                                                                                                                                               
-rw-rw-r-- 1 stas stas 15494 Feb  1 21:03 handle_ipynb_magics.py                                                                                                                                                                            
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 __init__.cpython-312-x86_64-linux-gnu.so                                                                                                                                                          
-rw-rw-r-- 1 stas stas 51644 Feb  1 21:03 __init__.py                                                                                                                                                                                       
-rwxrwxr-x 1 stas stas  8432 Feb  1 21:03 linegen.cpython-312-x86_64-linux-gnu.so                                                                                                                                                           
-rw-rw-r-- 1 stas stas 70488 Feb  1 21:03 linegen.py                                                                                                                                                                                        
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 lines.cpython-312-x86_64-linux-gnu.so                                                                                                                                                             
-rw-rw-r-- 1 stas stas 39620 Feb  1 21:03 lines.py                                                                                                                                                                                          
-rw-rw-r-- 1 stas stas    47 Feb  1 21:03 __main__.py                                                                                                                                                                                       
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 mode.cpython-312-x86_64-linux-gnu.so                                                                                                                                                              
-rw-rw-r-- 1 stas stas  9065 Feb  1 21:03 mode.py                                                                                                                                                                                           
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 nodes.cpython-312-x86_64-linux-gnu.so                                                                                                                                                             
-rw-rw-r-- 1 stas stas 30418 Feb  1 21:03 nodes.py                                                                                                                                                                                          
-rwxrwxr-x 1 stas stas  8432 Feb  1 21:03 numerics.cpython-312-x86_64-linux-gnu.so                                                                                                                                                          
-rw-rw-r-- 1 stas stas  1655 Feb  1 21:03 numerics.py                                                                                                                                                                                       
-rw-rw-r-- 1 stas stas  3933 Feb  1 21:03 output.py                                                                                                                                                                                         
-rwxrwxr-x 1 stas stas  8432 Feb  1 21:03 parsing.cpython-312-x86_64-linux-gnu.so                                                                                                                                                           
-rw-rw-r-- 1 stas stas  8621 Feb  1 21:03 parsing.py                                                                                                                                                                                        
drwxrwxr-x 2 stas stas  4096 Feb  1 21:03 __pycache__                                                                                                                                                                                       
-rw-rw-r-- 1 stas stas     0 Feb  1 21:03 py.typed                                                                                                                                                                                          
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 ranges.cpython-312-x86_64-linux-gnu.so                                                                                                                                                            
-rw-rw-r-- 1 stas stas 19704 Feb  1 21:03 ranges.py                                                                                                                                                                                         
-rw-rw-r-- 1 stas stas  3452 Feb  1 21:03 report.py                                                                                                                                                                                         
drwxrwxr-x 3 stas stas  4096 Feb  1 21:03 resources                                                                                                                                                                                         
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 rusty.cpython-312-x86_64-linux-gnu.so                                                                                                                                                             
-rw-rw-r-- 1 stas stas   557 Feb  1 21:03 rusty.py                                                                                                                                                                                          
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 schema.cpython-312-x86_64-linux-gnu.so                                                                                                                                                            
-rw-rw-r-- 1 stas stas   431 Feb  1 21:03 schema.py                                                                                                                                                                                         
-rwxrwxr-x 1 stas stas  8432 Feb  1 21:03 strings.cpython-312-x86_64-linux-gnu.so                                                                                                                                                           
-rw-rw-r-- 1 stas stas 13220 Feb  1 21:03 strings.py                                                                                                                                                                                        
-rwxrwxr-x 1 stas stas  8424 Feb  1 21:03 trans.cpython-312-x86_64-linux-gnu.so                                                                                                                                                             
-rw-rw-r-- 1 stas stas 95191 Feb  1 21:03 trans.py                                                                                                                                                                                          
-rwxrwxr-x 1 stas stas  8440 Feb  1 21:03 _width_table.cpython-312-x86_64-linux-gnu.so                                                                                                                                                      
-rw-rw-r-- 1 stas stas 10748 Feb  1 21:03 _width_table.py

It's on old and somewhat slow Intel(R) Core(TM) i3-8145U CPU @ 2.10GHz running Ubuntu 22.04, but I rarely encounter any performance problems using it for dev, so if black runs so much faster for you - maybe something is wrong with compilation targeting my CPU (or incompatible wheels? I'll try to install from source no, still 6-8 s over 3 runs with a locally compiled version from psf/black master)...

@cdce8p
Copy link
Collaborator Author

cdce8p commented Feb 11, 2025

@sterliakov Just ran your test on my machine (M2 Macbook Pro). It took 3.4s to reformat mypy/checker.py.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 11, 2025

I ran Black directly, not via pre-commit, and I had only edited mypy/checker.py since the previous Black run (Black is incremental). When I run it via pre-commit, it's a bit slower, but not by much. I used pre-commit run black --all-files. I'm mostly interested in the incremental runtime when a few files have been modified.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 11, 2025

Anyway, this looks pretty quick -- I measured about 0.6s seconds of extra when doing an incremental pre-commit run.

The question would be which part is actually slow for you?

"Lint GitHub Actions workflow files" feels annoyingly slow when using python runtests.py lint, especially considering that the files are changes quite rarely:

$ time pre-commit run actionlint --all-files
Lint GitHub Actions workflow files.......................................Passed

real	0m5.522s
user	0m43.927s
sys	0m1.235s

Everything else is kind of reasonable.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Feb 12, 2025

The question would be which part is actually slow for you?

"Lint GitHub Actions workflow files" feels annoyingly slow when using python runtests.py lint, especially considering that the files are changes quite rarely

Didn't know that runtests.py lint runs pre-commit with --all-files. I usually run pre-commit before commit, so I don't find it necessary to check the whole code base again.

Anyway, this looks pretty quick -- I measured about 0.6s seconds of extra when doing an incremental pre-commit run.

Anything blocking this PR then?

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

The false positive rate looks good and performance is reasonable, so let's give this a try!

@JukkaL JukkaL merged commit 8bdc4af into python:master Feb 12, 2025
18 checks passed
@cdce8p cdce8p deleted the add-codespell branch February 12, 2025 10:45
ericmarkmartin pushed a commit to ericmarkmartin/mypy that referenced this pull request Feb 19, 2025
Replaces python#18387

Use codespell in pre-commit to detect spelling mistakes, see python#18642.
Ignore test and typeshed folders.
ericmarkmartin pushed a commit to ericmarkmartin/mypy that referenced this pull request Feb 19, 2025
Replaces python#18387

Use codespell in pre-commit to detect spelling mistakes, see python#18642.
Ignore test and typeshed folders.
x612skm pushed a commit to x612skm/mypy-dev that referenced this pull request Feb 24, 2025
Replaces python#18387

Use codespell in pre-commit to detect spelling mistakes, see python#18642.
Ignore test and typeshed folders.
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.

3 participants