spyder-ide / qtpy

Provides an uniform layer to support PyQt5, PySide2, PyQt6, PySide6 with a single codebase
MIT License
973 stars 151 forks source link

Type hints not working for `qtpy` specific imports #447

Closed krokosik closed 1 year ago

krokosik commented 1 year ago

Hi,

I want to start an incremental migration from PyQt5 using your amazing project. However, I immediately hit an obstacle, when renaming my imports. The problem is with the abstracted Signal, Slot etc. classes. Their type hints are not picked up by VSCode's intellisense, which is a bit of a deal breaker for me :/

I looked into your QtCore file and it seems that this is caused by exporting Signal as both pyqtSignal from PyQt5 and Unknown from other packages. When I comment out other imports, I get the correct type, but I'm not familiar enough with Python typing hacks to figure out how to solve it. Is it even possible? I would very much like this feature and could submit a PR if you have some advice what to try.

dalthviz commented 1 year ago

Hi @wkrasnicki thank you for the feedback! Not totally sure how to proceed here (my knowledge about Python typings checking is quite limited to be honest) but which are the imports you are commenting out to enable the type checking to work? Maybe we just need to add a if not TYPE_CHECKING validation to such lines to ensure the typing works but also still have the actual imports?

Any other info is greatly appreciated! Let us know!

krokosik commented 1 year ago

Hello, I have produced a minimal example of the issue. It requires creating 3 files and assumes only PyQt5 is installed.

# env.py
PYQT5 = 1 == 1
PYQT6 = 1 == 2

# qtcore.py
from .env import PYQT6, PYQT5

if PYQT5:
    from PyQt5.QtCore import pyqtSignal as Signal
elif PYQT6:
    from PyQt6.QtCore import pyqtSignal as Signal

# main.py
from .qtcore import Signal

Signal()

When launched in VSCode with basic type checking, I expect this to show no red lines from type checking. There is a minor difference in the actual qtpy, where the variables from env.py are of type Literal[False] and Literal[True] respectively, but changing them to bool did not help, so here I am also using bools. I'm trying out different things, but no progress so far.

dalthviz commented 1 year ago

Thank you for the info @wkrasnicki ! I checked and I'm not seeing any red lines 🤔 Just in case, in your actual usage of QtPy do you have the config for mypy and QtPy set on VSCode? For more info you can check: https://github.com/spyder-ide/qtpy/blob/master/README.md#type-checker-integration (more specifically the link to the Pyright config comment).

Let us now if the info above helps!

krokosik commented 1 year ago

Ahh yes, that's it. The link to Pyright was a bit hidden, especially that I forgot about Pylance using it under the hood. +1 for the solution, but it is incorrect. This is how you pass multiple values in pyproject.toml:

[tool.pyright]
defineConstant = { PYQT5 = true , PYSIDE6 = false , PYSIDE2 = false , PYQT6 = false }

The one from the linked comment doesn't work and is even highlighted as incorrect config. Why is Pyright omitted from the README even though MyPy isn't?

dalthviz commented 1 year ago

Awesome @wkrasnicki glad to know you manage to config things in your setup! Regarding the README, for sure there are quite a few opportunities for improvement :) Do you think giving the Pyright info its own title/section could be better, right? For sure at least updating the info regarding the current way to config Pyright needs to be done. If you want to help us with that let us know!

krokosik commented 1 year ago

Hi @dalthviz I've created a PR with proposed changes. Let me know what you think :)

ccordoba12 commented 1 year ago

Closing as fixed by #450.