gh-109413: Add a custom script for running mypy on libregrtest #109464

AlexWaygood · 2023-09-15T19:02:19Z

Currently, the only way to run mypy on Lib/test/libregrtest is to cd into Lib/test and then run mypy --config-file libregrtest/mypy.ini. This isn't ideal, as invoking mypy like this means that all things imported from test.support are inferred by mypy as being of type Any. That usually just means that you get some "false negatives" when it comes to type checking, but for us, it also means that we get a false-positive error:

libregrtest\single.py:57: error: Expected type in class pattern; found "Any"  [misc]
            case TestStats():
                 ^~~~~~~~~~~

This PR adds a custom script for invoking mypy on libregrtest, so that mypy can see the classes and functions inside Lib/test/support/ when it's run on libregrtest. A custom script is needed, as mypy gets very confused generally about the Lib/ directory -- it thinks the whole directory is "shadowing the stdlib". This means that mypy can only be invoked from very specific locations in the CPython repo, or it "sees" everything that's in the Lib/ directory, panics at all the things it doesn't understand, and refuses to do any type-checking at all.

The workaround this script uses is to copy Lib/test/support into a temporary directory and teach mypy about the existence of test.support (without revealing the existence of the rest of the Lib/ directory) using the MYPYPATH environment variable.

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

AlexWaygood · 2023-09-15T19:03:22Z

Lib/test/support/__init__.py

Revealing the existence of this module to mypy necessitated fixing a few typing-related things to do with this module

AlexWaygood · 2023-09-15T19:03:53Z

Lib/test/support/__init__.py

+class WarningsPrinter:
+    def __init__(self, func: Callable[[str], None]) -> None:
+        self.func = func
+        # bpo-39983: Store the original sys.stderr at Python startup to be able to
+        # log warnings even if sys.stderr is captured temporarily by a test.
+        self.orig_stderr = sys.stderr
+
+    def __call__(self, msg: str) -> None:
+        return self.func(msg)
+
+
+@WarningsPrinter


Mypy doesn't like assigning attributes to functions, unfortunately; it much prefers the idea of a custom callable class

(This is the solution you said you preferred in #109382 (comment) :-)

AlexWaygood · 2023-09-15T19:04:36Z

Tools/scripts/run_mypy_on_regrtest.py

@vstinner: where do you think this script should go?

Where I have it now?

Inside Lib/test/libregrtest?

Somewhere else?

Also: thoughts on the name? It's a bit long and clunky right now...

I would prefer to put it in Tools/build/. I would be fine with Lib/test/libregrtest.

hauntsaninja · 2023-09-16T08:17:01Z

(for what it's worth, I think we fixed the Expected type in class pattern; found "Any" diagnostic in python/mypy#14479 )

AlexWaygood · 2023-09-16T12:23:58Z

(for what it's worth, I think we fixed the Expected type in class pattern; found "Any" diagnostic in python/mypy#14479 )

Ah, thank you! I thought I remembered this coming up recently, but I couldn't find the reference when I searched for it.

Hmm, @vstinner, that makes me even less enthused about this approach. I know you said you'd prefer a custom script so that mypy would be able to see the types inside test.support, but to me, this script feels like a hack that's really awkward to invoke. Having everything from test.support being inferred as Any isn't ideal, but it's not the end of the world either, and the only false positive we'd be fixing here is going to go away in an upcoming mypy release anyway. I think the cure might be worse than the disease in this case.

vstinner

Sorry @AlexWaygood, I'm not sure that I understood the alternative that you are proposing. No script? Can you maybe propose a PR implementating the alternative?

vstinner · 2023-09-16T16:58:03Z

Tools/scripts/run_mypy_on_regrtest.py

I would prefer to put it in Tools/build/. I would be fine with Lib/test/libregrtest.

AlexWaygood · 2023-09-17T16:17:21Z

Sorry @AlexWaygood, I'm not sure that I understood the alternative that you are proposing. No script? Can you maybe propose a PR implementating the alternative?

Yeah, the alternative would be to just invoke mypy directly by cding into Lib/test and running mypy --config-file libregrtest/mypy.ini -- you can do that now, although there are still some mypy errors. And if #109518 is merged, we won't even need to cd into Lib/test before invoking mypy -- we'll just be able to run mypy --config-file Lib/test/libregrtest/mypy.ini from the repo root to invoke mypy. It feels like a simpler and less hacky approach to me than a custom script.

The only disadvantage from that approach is that everything imported from test.support will be inferred by mypy as having type Any. But that's honestly not the worst thing in the world; I can live with that. There's a bug in mypy 1.5 that means that we would get a single false positive as a result of everything from test.support being inferred as Any. But the bug will be fixed in mypy 1.7 (#109464 (comment)), and in the meantime, we could just # type: ignore it.

vstinner · 2023-09-18T13:07:57Z

And if #109518 is merged, we won't even need to cd into Lib/test before invoking mypy

It got merged. Do you want to propose an alternative PR which doesn't use temporary directory and copytree()?

AlexWaygood · 2023-11-30T17:41:54Z

And if #109518 is merged, we won't even need to cd into Lib/test before invoking mypy

It got merged. Do you want to propose an alternative PR which doesn't use temporary directory and copytree()?

yes

Add a custom script for running mypy on libregrtest

770e39f

AlexWaygood requested a review from vstinner September 15, 2023 19:02

bedevere-app bot added the awaiting review label Sep 15, 2023

bedevere-app bot mentioned this pull request Sep 15, 2023

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

Open

AlexWaygood added the skip news label Sep 15, 2023

AlexWaygood commented Sep 15, 2023

View reviewed changes

vstinner reviewed Sep 16, 2023

View reviewed changes

Tools/scripts/run_mypy_on_regrtest.py Outdated

Copy link

Member

vstinner Sep 16, 2023

Choose a reason for hiding this comment

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

I would prefer to put it in Tools/build/. I would be fine with Lib/test/libregrtest.

AlexWaygood added 2 commits September 18, 2023 12:51

Merge branch 'main' into mypy-regrtest-script

50df501

Updates now python#109518 is merged

1806074

AlexWaygood closed this Nov 30, 2023

AlexWaygood deleted the mypy-regrtest-script branch November 30, 2023 17:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-109413: Add a custom script for running mypy on libregrtest #109464

gh-109413: Add a custom script for running mypy on libregrtest #109464

AlexWaygood commented Sep 15, 2023 •

edited by bedevere-app bot

Loading

AlexWaygood Sep 15, 2023

AlexWaygood Sep 15, 2023

AlexWaygood Sep 15, 2023

AlexWaygood Sep 15, 2023

vstinner Sep 16, 2023

hauntsaninja commented Sep 16, 2023

AlexWaygood commented Sep 16, 2023

vstinner left a comment

vstinner Sep 16, 2023

AlexWaygood commented Sep 17, 2023 •

edited

Loading

vstinner commented Sep 18, 2023

AlexWaygood commented Nov 30, 2023

gh-109413: Add a custom script for running mypy on libregrtest #109464

gh-109413: Add a custom script for running mypy on libregrtest #109464

Conversation

AlexWaygood commented Sep 15, 2023 • edited by bedevere-app bot Loading

AlexWaygood Sep 15, 2023

Choose a reason for hiding this comment

AlexWaygood Sep 15, 2023

Choose a reason for hiding this comment

AlexWaygood Sep 15, 2023

Choose a reason for hiding this comment

AlexWaygood Sep 15, 2023

Choose a reason for hiding this comment

vstinner Sep 16, 2023

Choose a reason for hiding this comment

hauntsaninja commented Sep 16, 2023

AlexWaygood commented Sep 16, 2023

vstinner left a comment

Choose a reason for hiding this comment

vstinner Sep 16, 2023

Choose a reason for hiding this comment

AlexWaygood commented Sep 17, 2023 • edited Loading

vstinner commented Sep 18, 2023

AlexWaygood commented Nov 30, 2023

AlexWaygood commented Sep 15, 2023 •

edited by bedevere-app bot

Loading

AlexWaygood commented Sep 17, 2023 •

edited

Loading