Skip to content
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.

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

Type annotations #417

Open
TilmanK opened this issue Apr 20, 2022 · 15 comments
Open

Type annotations #417

TilmanK opened this issue Apr 20, 2022 · 15 comments

Comments

@TilmanK
Copy link
Contributor

TilmanK commented Apr 20, 2022

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
Copy link
Member

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
Copy link
Contributor Author

TilmanK commented Apr 20, 2022

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

@nicoddemus
Copy link
Member

the dynamic use of the Qt library?

Hmm sorry can you be more specific?

@TilmanK
Copy link
Contributor Author

TilmanK commented Apr 22, 2022

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
Copy link
Member

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
Copy link
Contributor Author

TilmanK commented Apr 22, 2022

We could do something like.

if TYPE_CHECKING:
    qt_api = PyQt6

Maybe...

@nicoddemus
Copy link
Member

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
Copy link
Contributor Author

TilmanK commented Apr 23, 2022

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
Copy link
Member

nicoddemus commented Apr 23, 2022

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
Copy link
Member

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
Copy link
Contributor Author

TilmanK commented Apr 25, 2022

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

@nicoddemus
Copy link
Member

Great! 😁

@The-Compiler
Copy link
Member

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
Copy link

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
Copy link
Contributor Author

TilmanK commented Jun 18, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants