Skip to content

gh-109413: Run mypy on libregrtest in CI #112558

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 30, 2023
Merged

gh-109413: Run mypy on libregrtest in CI #112558

merged 3 commits into from
Nov 30, 2023

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Nov 30, 2023 •
edited
Loading

  • Fix remaining mypy errors
  • Add a CI check to stop typing issues from regressing in the future

@AlexWaygood AlexWaygood changed the title Run mypy on libregrtest in CI gh-109413: Run mypy on libregrtest in CI Nov 30, 2023
@bedevere-app bedevere-app bot mentioned this pull request Nov 30, 2023
Open
@AlexWaygood AlexWaygood marked this pull request as ready for review November 30, 2023 17:44
@AlexWaygood AlexWaygood requested a review from vstinner November 30, 2023 17:44
AlexWaygood
AlexWaygood commented Nov 30, 2023
View reviewed changes
Lib/test/libregrtest/results.py
self.testsuite_xml: list[str] = []
self.testsuite_xml: list = []
Copy link
Member Author

Choose a reason for hiding this comment

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

This annotation is incorrect; leaving it as it is causes mypy to issue 5 or so errors in various places. We should correct the annotation, ideally, but for now making it more vague gets rid of the errors from mypy. I prefer to defer fixing it properly to a future PR, for now.

hugovk
hugovk approved these changes Nov 30, 2023
View reviewed changes
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

For the new stuff that mypy doesn't know about, can they be ignored via mypy.ini?

If so, that would be cleaner, no need to temporarily add the extra comments in py files.

Otherwise looks good.

@AlexWaygood @hugovk
cleanup comments
5160db5 
Co-authored-by: Hugo van Kemenade <[email protected]>
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Nov 30, 2023 •
edited
Loading

For the new stuff that mypy doesn't know about, can they be ignored via mypy.ini?

If so, that would be cleaner, no need to temporarily add the extra comments in py files.

I don't think so, unfortunately :(

Mypy's config supports ignoring all errors in a file, or ignoring all errors of a certain category in a file. But I don't think there's a config-file option to ignore all errors due to specific functions or parameters not existing, unfortunately :/

(And, to be fair, that comes up more in CPython than elsewhere! We're literally writing the stdlib here, after all 😄)

vstinner
vstinner approved these changes Nov 30, 2023
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. These changes seem reasonable to me.

@AlexWaygood
Copy link
Member Author

Thanks both!

@AlexWaygood AlexWaygood merged commit 674c288 into python:main Nov 30, 2023
@AlexWaygood AlexWaygood deleted the mypy-libregrtest branch November 30, 2023 23:00
AlexWaygood added a commit to AlexWaygood/cpython that referenced this pull request Dec 1, 2023
@AlexWaygood
[3.12] libregrtest: Backport .py-file changes from python#112558
713b043
@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2023

GH-112605 is a backport of this pull request to the 3.12 branch.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Dec 1, 2023 •
edited
Loading

GH-112605 is a backport of this pull request to the 3.12 branch.

(I'm not actually backporting the change to the .yml file; we won't actually be running mypy on the 3.12 branch in CI. This just backports some of the changes to the .py files, to make future backports easier.)

@vstinner
Copy link
Member

vstinner commented Dec 1, 2023

Thanks for enabling mypy on libregrtest :-)

AlexWaygood added a commit that referenced this pull request Dec 1, 2023
@AlexWaygood
[3.12] gh-109413: libregrtest: Backport .py-file changes from #112558…
acc62db 
… (#112605)
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
@AlexWaygood @miss-islington
[3.12] pythongh-109413: libregrtest: Backport .py-file changes from p…
8866dea 
…ythonGH-112558 (pythonGH-112605)

(cherry picked from commit acc62db)

Co-authored-by: Alex Waygood <[email protected]>
AlexWaygood added a commit that referenced this pull request Dec 1, 2023
@miss-islington @AlexWaygood
[3.11] [3.12] gh-109413: libregrtest: Backport .py-file changes from …
720f5bf 
…GH-112558 (GH-112605) (#112607)

[3.12] gh-109413: libregrtest: Backport `.py`-file changes from GH-112558 (GH-112605)
(cherry picked from commit acc62db)

Co-authored-by: Alex Waygood <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@AlexWaygood @hugovk @aisk
pythongh-109413: Run mypy on libregrtest in CI (python#112558)
c04b0eb 
Co-authored-by: Hugo van Kemenade <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@AlexWaygood @hugovk @Glyphack
pythongh-109413: Run mypy on libregrtest in CI (python#112558)
8d89410 
Co-authored-by: Hugo van Kemenade <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

@hugovk hugovk hugovk approved these changes

@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexWaygood @vstinner @hugovk