pytest-qt icon indicating copy to clipboard operation
pytest-qt copied to clipboard

Type annotations

Open TilmanK opened this issue 3 years ago • 15 comments

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)…

TilmanK avatar Apr 20 '22 15:04 TilmanK

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.

nicoddemus avatar Apr 20 '22 17:04 nicoddemus

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

TilmanK avatar Apr 20 '22 17:04 TilmanK

the dynamic use of the Qt library?

Hmm sorry can you be more specific?

nicoddemus avatar Apr 20 '22 17:04 nicoddemus

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

TilmanK avatar Apr 22 '22 20:04 TilmanK

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.

nicoddemus avatar Apr 22 '22 21:04 nicoddemus

We could do something like.

if TYPE_CHECKING:
    qt_api = PyQt6

Maybe...

TilmanK avatar Apr 22 '22 21:04 TilmanK

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... :/

nicoddemus avatar Apr 22 '22 22:04 nicoddemus

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...

TilmanK avatar Apr 23 '22 04:04 TilmanK

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 avatar Apr 23 '22 11:04 nicoddemus

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.

nicoddemus avatar Apr 25 '22 10:04 nicoddemus

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

TilmanK avatar Apr 25 '22 11:04 TilmanK

Great! 😁

nicoddemus avatar Apr 25 '22 11:04 nicoddemus

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.

The-Compiler avatar Apr 30 '22 14:04 The-Compiler

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.

adam-grant-hendry avatar Jun 15 '22 17:06 adam-grant-hendry

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

TilmanK avatar Jun 18 '22 19:06 TilmanK