pytest-dev / pytest-qt

pytest plugin for Qt (PyQt5/PyQt6 and PySide2/PySide6) application testing
https://pytest-qt.readthedocs.io
MIT License
402 stars 71 forks source link

Type annotations #417

Open TilmanK opened 2 years ago

TilmanK commented 2 years ago

I stumbled upon this when setting a new project where I haven't configured mypy yet to just ignore pytest-qt: There are no type annotations.

Is there a reason from this apart from “It's not there because no one did it yet”? It would be a nice-to-have thing, since the earliest mayor Python version supported is 3.6 and pytest itself has them (as far as I remember)…

nicoddemus commented 2 years ago

Hi @TilmanK!

“It's not there because no one did it yet”?

Mainly that only. I'm a big fan of having type annotations (even aiming for full type annotations), but nobody has stopped to bring them to pytest-qt yet.

TilmanK commented 2 years ago

Hmm, any idea how to address the dynamic use of the Qt library? Maybe using TypeVars in some way?

nicoddemus commented 2 years ago

the dynamic use of the Qt library?

Hmm sorry can you be more specific?

TilmanK commented 2 years ago

Sure. Let's take QtBot.addWidget() as an example:

def addWidget(self, widget, *, before_close_func=None):

How do I annotate that? I can use widget: qt_api.QtWidgets.QWidget. Doing so results in an error: Name "qt_api.QtWidgets.QWidget" is not defined

nicoddemus commented 2 years ago

Oh I see thanks.

Hmm that's tricky, not sure how to overcome that: qt_api by design only defines the symbols at runtime (during pytest_configure), so there's no way to do that at static time.

TilmanK commented 2 years ago

We could do something like.

if TYPE_CHECKING:
    qt_api = PyQt6

Maybe...

nicoddemus commented 2 years ago

I thought of that, the problem is that the choice of using PyQt or PySide is defined at realtime too, so that snippet won't work for PySide users... :/

TilmanK commented 2 years ago

I thought of that, the problem is that the choice of using PyQt or PySide is defined at realtime too, so that snippet won't work for PySide users... :/

Yes, I know. Sorry, the code snippet isn't clear. Of course, I don't want to hard-wire the API to PyQt6, I was just thinking about a first approach to tell mypy which package to use...

nicoddemus commented 2 years ago

I was just thinking about a first approach to tell mypy which package to use...

Indeed. However that selection is done at runtime by design (checking an environment variable, or the configuration in pytest.ini file), which are ultimately runtime decisions, so mypy can't know about them at static time. 🤔

nicoddemus commented 2 years ago

Just thinking about this: we might decide to type any qt-related class as Any for now, and properly type everything else. Having a widget: Any parameter is not so bad for now if we type the other parameters. If we find a solution down the road, we can then improve the typing coverage accordingly.

TilmanK commented 2 years ago

I already started to work on a PR doing exactly this 😊

nicoddemus commented 2 years ago

Great! 😁

The-Compiler commented 2 years ago

FWIW, the way other Qt wrappers seem to approach this (qtpy, qts) is that they have a CLI supplying --always-true and --always-false options to mypy.

adam-grant-hendry commented 2 years ago

Thanks for linking me to the discussion here @nicoddemus ! I'll throw my hat in the ring and put up a PR.

From our discussion, we agreed adding type hints inline to the source was preferred over using stubs(*.pyi files), which I 100% support. I think this is the best approach.

I have one file hinted as of yet. Per PEP 561, you can add a line "partial\n" to the "py.typed" marker to indicate the package is partially stubbed. I'll add this so we can add type hints one-by-one to each module as we create them and have PRs for individual modules as opposed to a giant PR that type hints the whole code base at one go.

TilmanK commented 2 years ago

Go for it, I currently can find the time to do this anyway...