python / cpython

The Python programming language
https://www.python.org
Other
62.42k stars 29.97k forks source link

libregrtest: Improve type hints; explore running mypy in CI #109413

Open AlexWaygood opened 1 year ago

AlexWaygood commented 1 year ago

Feature or enhancement

Proposal:

The code in Lib/test/libregrtest already has a smattering of type hints in it, but doesn't currently pass type checking. @vstinner, the primary maintainer of libregrtest, expressed interest to me in seeing whether we could run mypy on libregrtest in CI.

This issue is for tracking progress towards that.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Linked PRs

AlexWaygood commented 1 year ago

We currently do not have any projects inside the Lib/ directory that are mypy-checked in CI: currently, all the mypy-checked projects we have in CPython live in the Tools/. If we want to actually run mypy on libregrtest in CI, I think it should be discussed on Discourse first, to check if anybody has any objections to this idea.

encukou commented 3 months ago

What should I do when I have a pull request that adds a new argument to a stdlib function, and also makes libregrtest use that argument? See here: https://github.com/python/cpython/actions/runs/9521883482/job/26250277500?pr=120520

AlexWaygood commented 3 months ago

What should I do when I have a pull request that adds a new argument to a stdlib function, and also makes libregrtest use that argument? See here: https://github.com/python/cpython/actions/runs/9521883482/job/26250277500?pr=120520

Add a type: ignore comment for now to the line where you're using the new parameter, and add a comment saying why you're ignoring the mypy error (because mypy doesn't know about the parameter yet).

If you want to be very nice, you could also make a PR to typeshed adding the new parameter to typeshed's stdlib stubs on py314+ after you've merged the CPython PR (mypy uses typeshed as the single source of truth for the stdlib). But that's definitely not compulsory, just if you feel like it. We'll get to it on our own eventually anyway over at typeshed.

encukou commented 3 months ago

Thank you! Here the argument is only meant for internal use, so I don't think a typeshed PR is necessary.

However, it would be nice to send a PR to the Devguide. I can write one up, but if you have time for it, you could probably word it better.

AlexWaygood commented 3 months ago

Not sure I have the time right now for a devguide PR (definitely not until the weekend), but would happily review one! This has come up before, so agree that better docs are probably needed.