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 setup.py to selfcheck #18609

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add setup.py to selfcheck #18609

wants to merge 3 commits into from

Conversation

mr-c
Copy link
Contributor

@mr-c mr-c commented Feb 5, 2025

Enhance the setup.py types.

mypy's setup.py is used as inspiration for other Setuptools-using projects that want to produce mypyc compiled binary wheels. Therefore it should also be typechecked and held to a higher standard.

This comment has been minimized.

runtests.py Outdated
"-c",
"from mypy.main import main as mypy; "
"mypy(args=['--config-file', 'mypy_self_check.ini', '-p', 'mypy', '-p', 'mypyc']); "
"mypy(args=['--config-file', 'mypy_self_check.ini', 'setup.py'])",
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we should have two different type checks, self (which checks mypy) and self-packaging (which checks setup.py)?

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 also thought about splitting them, but then I saw the comment at https://github.com/python/mypy/pull/18609/files/72e1ff81760a6e50a2acba2d2a262ad409afdc25#diff-17019d8813f584a97bc5531faa1df3f1a590413d61a77d7d93db76df66163b63R53-R54

Each run should have tests that each take a roughly similar time to run.

To me, the check of just setup.py feels too short to have its own run.

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'm happy to take this PR in whatever direction the reviewers / mergers / maintainers wish, of course :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's a comment on the runtime of different pytest runs. Locally, I see:

(.venv) PS C:\Users\A5rocks\Documents\mypy> Measure-Command { python runtests.py self }


Days              : 0
Hours             : 0
Minutes           : 1
Seconds           : 42
Milliseconds      : 497
Ticks             : 1024979731
TotalDays         : 0.00118631913310185
TotalHours        : 0.0284716591944444
TotalMinutes      : 1.70829955166667
TotalSeconds      : 102.4979731
TotalMilliseconds : 102497.9731



(.venv) PS C:\Users\A5rocks\Documents\mypy> Measure-Command { python runtests.py pytest-fast }


Days              : 0
Hours             : 0
Minutes           : 3
Seconds           : 38
Milliseconds      : 93
Ticks             : 2180936287
TotalDays         : 0.00252423181365741
TotalHours        : 0.0605815635277778
TotalMinutes      : 3.63489381166667
TotalSeconds      : 218.0936287
TotalMilliseconds : 218093.6287

(and that's on a system with 6 cores/12 threads, meaning pytest is running in parallel and mypy isn't.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Shall I squash+rebase and force push?

mr-c added a commit to mr-c/mypy that referenced this pull request Feb 6, 2025

This comment has been minimized.

mr-c and others added 3 commits February 6, 2025 17:10
Enhance the setup.py types.

mypy's setup.py is used as inspiration for other setuptools using Python
projects that want to produce mypyc compiled binary wheels. Therefore it
should also be typechecked and held to a higher standard.
@mr-c mr-c force-pushed the typecheck_setup.py branch from f8f77b8 to b580485 Compare February 6, 2025 16:10
Copy link
Contributor

github-actions bot commented Feb 6, 2025

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

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