Type checking and auto-completion with PyQt6 (and PyQt5)

[toofar]

This is split from #7202 and will probably make #7128 and the WIP V2 of that PR obsolete, as it looks like re-writing everything is in fact not necessary.

What I think the next steps on this workstream are:

1. check my working, I've only been looking at this stuff again for a couple of days so I'm probably making some wrong assumptions
2. agree on the way forward for how to do type checking on CI (although I think that's a no-brainier at this point) and how much we need to do before merging (eg just get PyQt5 working again or both?)
3. fix lots of issues mypy is raising, hopefully I can get some help with that, maybe we'll make a GH discussion to talk about the specific issues as they come up instead of trying to use issues or PRs for talking about the 200 odd issues
4. figure out a bit more about enabling IDE autocompletion and what we can do to make it more robust for people with different setups

Discussion of specific decisions, issues, subtasks etc of this issue should be done in a threaded fashion in the discussion: #7372

Following is a large amount of words on what we are trying to achieve, some backing on complications specific to out setup, simplified example, some background on prior art and solutions that don't work for us. The IDE part is still a bit of a mess.

Hopefully it's useful for people who haven't been across this stuff yet. Hopefully we can clean it up and refer back to it when we are working through the mypy issues.

---

What

Look at options to get type checkers and autocompletion working with conditional imports.

Why

With the move to support Qt6 we would like to introduce an abstraction mechanism to support multiple Qt wrappers for python. The major reasons are as follows:

• we would like to continue supporting Qt5 for a while (Drop Qt < 5.15 #7091); moving Qt5 support to a stable branch is still an option but:
• adding an abstraction layer around the Qt bindings will make it easier to explore PySide, the official Qt projects bindings

Strategies for supporting multiple Qt bindings

There are a few strategies we have seen about the place for supporting multiple Qt bindings. One of the reasons we might choose one over the other is how easy or hard they are to get working with static type checkers. Generally we would prefer to go with the qts style strategy because that is the one that is the one that is already implemented on the qt6-v2 and having to go back and re-do stuff sucks.

expand for discussion of the different strategies with examples and pros/cons

qts/machinery style

Inspired by: https://github.com/python-qt-tools/qts/tree/main/src/qts
Uses wildcard import from Qt submodules into modules under our namespace
Includes some patching and renaming after import to present a consistent API across the backends

difficult for type checkers:

• wildcard imports
• conditional imports
• patching after importing

difficult for humans:

• slower startup

example:

file: machinery.py (same in all examples)

def select_wrapper():
    "some runtime stuff to detect which bindings to use"

WRAPPER = select_wrapper()
USE_PYQT6 = WRAPPER == "PyQt6"
USE_PYQT5 = WRAPPER == "PyQt5"

file: core.py

from . import machinery

if machinery.USE_PYQT5:
    from PyQt5.QtCore import *
elif machinery.USE_PYQT6:
    from PyQt6.QtCore import *

file: application.py

from core import pyqtSignal
import machinery

if machinery.USE_PYQT6:
    # do any backend specific stuff
foo = pyqtSignal()

rewrite style

Import modules into single file to avoid wildcard import
Have to change how we access Qt attributes since we can't do import from
Includes some patching and renaming after import to present a consistent API across the backends

difficult for type checkers:

• conditional imports
• patching after importing

difficult for humans:

• going to make all the PR conflict on more than just imports
  • we can attempt re-writing tools to help people rebase/merge their branches but the edge cases are a huge pain, nothing really complete out of the box that can help us here atm

example:

file: machinery.py (same in all examples)

def select_wrapper():
    "some runtime stuff to detect which bindings to use"

WRAPPER = select_wrapper()
USE_PYQT6 = WRAPPER == "PyQt6"
USE_PYQT5 = WRAPPER == "PyQt5"

file: qt.py

from . import machinery

if machinery.USE_PYQT5:
    from PyQt5 import QtCore
    .. and other Qt modules we use are imported into this file too ...
elif machinery.USE_PYQT6:
    from PyQt6 import QtCore
    .. and other Qt modules we use are imported into this file too ...

file: application.py

from qt import core
import machinery

if machinery.USE_PYQT6:
    # do any backend specific stuff
foo = core.pyqtSignal()  # <-- this is the kind of thing we would have to re-write

generated

Calibre style: https://github.com/kovidgoyal/calibre/tree/master/src/qt
Similar to the machinery/qts style
Auto generated files of explicit imports and renames rather than wildcard import
Auto generated stub files for type checkers since actual imports are dynamic
Some strategies seen here could help mitigate slow startup with wildcard imports (eg autogenerate, or maintain, a big list of imports, instead of wildcard importing an even bigger list)

difficult for type checkers:

• auto generated type hints are only for primary bindings, if you use an alternate one the stubs would be a lie
  • (it doesn't actually support PyQt5 anymore, but you could add alternate versions of the name mappings)

example:

file: machinery.py (same in all examples)

def select_wrapper():
    "some runtime stuff to detect which bindings to use"

WRAPPER = select_wrapper()
USE_PYQT6 = WRAPPER == "PyQt6"
USE_PYQT5 = WRAPPER == "PyQt5"

file: core.py

from . import machinery

# the mapping for the preferred binding will also be written to a .pyi file for type hints
LOOKUPS = {
  "PyQt5": {
      "pyqtSignal": "QtCore.pyqtSignal",
    },
    "PyQt6": {
      "pyqtSignal": "QtCore.pyqtSignal",
    },
}

__getattr__(name):
  return importlib.import(LOOKUPS[machinery.WRAPPER][name])

file: application.py (same as qts style)

from core import pyqtSignal
import machinery

if machinery.USE_PYQT6:
    # do any backend specific stuff
foo = pyqtSignal()

Conditional Imports

The main difficulty common to these strategies is the conditional imports. We have to hold mypy a bit carefully to help it reason about this situation.

some examples of attempts to help mypy reason about conditional imports without using the "compile time constants" command line arguments

Lets go over some examples of how mypy fails to deal with them and how we could help it.

This first one is a common enough pattern and works well enough at runtime. But not only is the second import explicitly called as re-using a variable name but mypy has no idea what to do with situation and so doesn't retain any type information for the core name.

try:
    from PyQt5 import QtCore as core
except ImportError:
    from PyQt6 import QtCore as core  # error: Name "core" already defined (by an import)
reveal_type(core)  # revealed type is Any

You can get around that "already defined" error by using a common variable like this. But mypy still doesn't know how to make sense of the situation, even within the branch where we do our best to remove ambiguity:

# QKeyCombination is new in Qt6
core = None
try:
    # importing as a seperate name and then assigning to a commong variable
    # avoids '"core" was already defined (by an import) errors'
    from PyQt5 import QtCore as core5
    core = core5
except ImportError:
    from PyQt6 import QtCore as core6
    core = core6
    assert core is core6  # attempts at type narrowing
    assert core.QKeyCombination
    reveal_type(core6.QKeyCombination)  # revealed type is PyQt6.QtCore.QKeyCombination
    reveal_type(core.QKeyCombination)  # revealed type is Any
assert core is not None  # narrow type to remove the Optional

You'll see some guides on the internet saying to do something like

if TYPE_CHECKING:
  from PyQt6 import QtCore as core
else:
  try:
    from PyQt6 import QtCore as core
  except ImportError:
    from PyQt5 import QtCore as core

As a way to make the type checker avoid the ambiguity. Most of those cases are to deal with cases where you import the exact same module/object from different places. In our case PyQt5 and PyQt6 are totally different modules, which just happen to have largely overlapping APIs. It might be a way forward for us but it limits the amount of coverage we would get from type checking while we support multiple backends.

Here is a situation that shows how mypy compares the signatures between imports. We can see that it did pick a type for the imported object (the first one, I have both bindings installed in this case) but it also raises an error because the two branches import objects with different signatures:

try:
    from PyQt6.QtCore import pyqtSignal
except ImportError:
    from PyQt5.QtCore import pyqtSignal  # error: Incompatible import of "pyqtSignal" (imported name has type "Type[PyQt5.QtCore.pyqtSignal]", local name has type "Type[PyQt6.QtCore.pyqtSignal]")

reveal_type(pyqtSignal)  # Revealed type is "def (*types: Any, *, name: builtins.str =) -> PyQt6.QtCore.pyqtSignal"

There is a good write-up on options for this case here: https://stackoverflow.com/questions/59037244/mypy-incompatible-import-error-for-conditional-imports
What it doesn't mention is if we can choose to ignore that error in our binding abstraction layer. I don't think we can but we can come back to that if it becomes a problem with a more fleshed out strategy.

The current approach that qutebrowser uses is structured to make it easy to guide mypy using its command line args and then having a single place in the code (or a small set of variables that are consistently used) that guides which path we go down.

Consider these two examples files:

file: machinery.py

def select_wrapper():
    "some runtime stuff to detect which bindings to use"

WRAPPER = select_wrapper()
# Booleans we can override for mypy at "compile time"
USE_PYQT6 = WRAPPER == "PyQt6"
USE_PYQT5 = WRAPPER == "PyQt5"

file: core.py

from . import machinery

if machinery.USE_PYQT5:
    from PyQt5.QtCore import *
elif machinery.USE_PYQT6:
    from PyQt6.QtCore import *

reveal_type(pyqtSignal)

Just running mypy normally causes it to throw up a bunch of errors about incompatible imports being imported as the same name:

$ mypy core.py
... lots of errors from the the stuff in the wildcard import like ...
machinery/core.py:6: error: Incompatible import of "pyqtSignal" (imported name has type "Type[PyQt6.QtCore.pyqtSignal]", local name has type "Type[PyQt5.QtCore.pyqtSignal]")
machinery/core.py:8: note: Revealed type is "def (*types: Any, *, name: builtins.str =) -> PyQt5.QtCore.pyqtSignal"
Found 168 errors in 1 file (checked 1 source file)

Since we are using centrally located variables to indicated which wrapper we are using we can use mypy's --always-true and --always-false arguments to cause it to pick certain paths.

$ mypy --always-true=USE_PYQT6 --always-false=USE_PYQT5 core.py
machinery/core.py:8: note: Revealed type is "def (*types: Any, *, name: builtins.str =) -> PyQt6.QtCore.pyqtSignal"
Success: no issues found in 1 source file

$ mypy --always-false=USE_PYQT6 --always-true=USE_PYQT5 core.py
machinery/core.py:8: note: Revealed type is "def (*types: Any, *, name: builtins.str =) -> PyQt5.QtCore.pyqtSignal"
Success: no issues found in 1 source file

You can also set these options in the configuration file like so:

file: .mypy.ini

[mypy]
always_true = USE_PYQT6,IS_QT6
always_false = USE_PYQT5,USE_PYSIDE2,USE_PYSIDE6,IS_QT5

We may want to set our preferred bindings as the default in the config file to make basic mypy invocations more useful. (discussion: #7372 (comment))

Use Cases

There are a couple of different use cases we have for type hints.

type checking in CI

We would like to enable mypy CI jobs again. Options:

1. only check types with Qt6
2. have separate runs for PyQt5 and PyQt6
3. try to do one run with both backends

Given the above it makes sense to try for (2), try to run type checking for both bindings, one at a time.
It's also possible to just set it up for one binding for now and add the other one later. Although given we have much less errors with PyQt5 than PyQt6 it would probably be just PyQt5 in that case. And we don't won't to leave PyQt6 uncovered for long or it'll just introduce more drift.

There is some work to do on that front:

• add/correct code that was added when we didn't have mypy running that just doesn't comply with type signatures of either backend ✔️
• go through the mypy output for each backend and start addressing the issues raised
  • PyQt5 only has 60 errors, most of them seem to be ones introduced while adding support for Qt6 (enum name changes and such) ✔️
  • PyQt6 has many more, about 200. A lot of these related to webkit, since there is no webkit module in Qt6 it is treated as Any and all the # ignore comments about it are being complained about. But they are needed with PyQt5, not sure how to deal with that.

IDE/Editor Autocompletion and Definitions

IDEs use type hints to provide autocompletion and find definitions for things. They have the same problem with multiple imports, they need to pick one import. We should hopefully be able to configure things to make the backend selection by IDEs stable. We may run into issues in the future managing installed backed, selected backends and mismatches between development environments and our CI setup.

VS Code is particularly interesting to support since it uses pylance for autocompletion which uses pyright as the backend.
Pyright is an alternative to mypy and some of the strategies above don't apply to it and it has different quirks.
The alternate LSP implementation that ships with VS Code, jedi, seems to have better autocompletion support in this case but it also appears to be significantly slower. There is apparently a mypy-lsp implementation but I have no idea how to get it working with VS Code.

For now we are shipping a pyrightconfig.json configuration file which supports defineConstant setting which is analogous to mypy's --allways-true/false settings (but better because it supports more than just bool). Like so:

{
  "defineConstant": {
    "USE_PYQT6": true,
    "USE_PYQT5": false,
    "USE_PYSIDE2": false,
    "USE_PYSIDE6": false,
    "IS_QT5": false,
    "IS_QT6": true
  }
}

Rambling notes on how pyright deals with conditional imports *without* the defineConstant settings.

For example, here is our current webenginecore (with a couple of reveal_type()s added at the bottom):

from . import machinery

if machinery.USE_PYQT5:
    from PyQt5.QtWebEngineCore import *
    from PyQt5.QtWebEngineWidgets import (
        QWebEngineSettings,
        QWebEngineProfile,
        QWebEngineDownloadItem as QWebEngineDownloadRequest,
        QWebEnginePage,
        QWebEngineCertificateError,
        QWebEngineScript,
        QWebEngineHistory,
        QWebEngineHistoryItem,
        QWebEngineScriptCollection,
        QWebEngineClientCertificateSelection,
        QWebEngineFullScreenRequest,
        QWebEngineContextMenuData as QWebEngineContextMenuRequest,
    )
    # FIXME:qt6 is there a PySide2 equivalent to those?
    from PyQt5.QtWebEngine import PYQT_WEBENGINE_VERSION, PYQT_WEBENGINE_VERSION_STR
elif machinery.USE_PYQT6:
    from PyQt6.QtWebEngineCore import *
elif machinery.USE_PYSIDE2:
    from PySide2.QtWebEngineCore import *
    from PySide2.QtWebEngineWidgets import (
        QWebEngineSettings,
        QWebEngineProfile,
        QWebEngineDownloadItem as QWebEngineDownloadRequest,
        QWebEnginePage,
        QWebEngineCertificateError,
        QWebEngineScript,
        QWebEngineHistory,
        QWebEngineHistoryItem,
        QWebEngineScriptCollection,
        QWebEngineClientCertificateSelection,
        QWebEngineFullScreenRequest,
        QWebEngineContextMenuData as QWebEngineContextMenuRequest,
    )
elif machinery.USE_PYSIDE6:
    from PySide6.QtWebEngineCore import *
else:
    raise machinery.UnknownWrapper()

reveal_type(QWebEngineHistory)
reveal_type(QWebEngineNotification)

A bunch of modules got moved from webenginewidgets to webenginecore with the Qt6 release (they took the opportunity of a major version bump to clean up their codebase a bit) so we are importing some stuff from widgets in Qt5 to our wrapper so that non-wrapper code only has to know about them in one place.

$ mypy --always-true=USE_PYQT6 --always-false=USE_PYQT5 webenginecore.py
machinery/webenginecore.py:50: note: Revealed type is "def (parent: Union[PyQt6.QtCore.QObject, None] =) -> PyQt6.QtWebEngineCore.QWebEngineHistory"
machinery/webenginecore.py:51: note: Revealed type is "def (parent: Union[PyQt6.QtCore.QObject, None] =) -> PyQt6.QtWebEngineCore.QWebEngineNotification"
Success: no issues found in 1 source file

$ mypy --always-false=USE_PYQT6 --always-true=USE_PYQT5 webenginecore.py
machinery/webenginecore.py:50: note: Revealed type is "def () -> PyQt5.QtWebEngineWidgets.QWebEngineHistory"
machinery/webenginecore.py:51: note: Revealed type is "def (parent: Union[PyQt5.QtCore.QObject, None] =) -> PyQt5.QtWebEngineCore.QWebEngineNotification"
Success: no issues found in 1 source file

$ pyright webenginecore.py
webenginecore.py:50:13 - information: Type of "QWebEngineHistory" is "Type[QWebEngineHistory] | Unknown | Unbound"
webenginecore.py:50:13 - error: "QWebEngineHistory" is possibly unbound (reportUnboundVariable)
webenginecore.py:51:13 - information: Type of "QWebEngineNotification" is "Type[QWebEngineNotification]"

Notice the difference between the two revealed types at the end.
QWebEngineNotification is revealed as Type[QWebEngineNotification] but QWebEngineHistory is revealed as Type[QWebEngineHistory] | Unknown | Unbound.
Tellingly, we also do currently get autocompletion for QWebEngineNotification and we don't get it for QWebEngineHistory.

In this case I don't have either of the PySide modules installed. After installing both of them:

webenginecore.py:41:9 - error: "QWebEngineClientCertificateSelection" is unknown import symbol (reportGeneralTypeIssues)
webenginecore.py:50:13 - information: Type of "QWebEngineHistory" is "Type[QWebEngineHistory]"
webenginecore.py:51:13 - information: Type of "QWebEngineNotification" is "Type[QWebEngineNotification]"

Suddenly there are no more possibly unbound variables and I also have autocompletion for QWebEngineHistory in VS Code!

So what is going on here? It seems the two failed imports (of PySide2 and PySide6) cause some possible interpretations of those variables to be tainted with Unknown or Unbound which cause pylance not get any type information from them.

Trying to narrow it down further it seems it needs either PySide2 OR PySide6 installed to get autocompletion for the moved modules. Why? Is it because I don't have type stubs for PyQt installed?

Use Library Code For Types = off
Type Checking Mode = off
PySide2 = installed
PySide6 = installed
PyQt5-stubs = installed
PyQt6-stubs = installed
QWebEngineHistory = Any

Use Library Code For Types = off
Type Checking Mode = *basic*
PySide2 = installed
PySide6 = installed
PyQt5-stubs = installed
PyQt6-stubs = installed
QWebEngineHistory = class
Go to Definition = PySide6

Use Library Code For Types = off
Type Checking Mode = *basic*
PySide2 = installed
PySide6 = *uninstalled*
PyQt5-stubs = installed
PyQt6-stubs = installed
QWebEngineHistory = class
Go to Definition = *PySide2*

Use Library Code For Types = off
Type Checking Mode = *basic*
PySide2 = *uninstalled*
PySide6 = *uninstalled*
PyQt5-stubs = installed
PyQt6-stubs = installed
QWebEngineHistory = *Unknown*
Go to Definition = *N/A*

So it seems if you have "type checking mode" set it'll use a PySide library for type things.

BUT, if you go to webenginecore.py and remove both PySide imports...

Use Library Code For Types = off
Type Checking Mode = off
PySide2 = N/A (removed support)
PySide6 = N/A (removed support)
PyQt5-stubs = installed
PyQt6-stubs = installed
QWebEngineHistory = class
Go to Definition = PyQt6 (from the proper install, not my stubs)

How bizarre. I could understand if any of the binding modules being missing caused errors. Or just PySide2 since that (and PyQt5) are have some stuff imported from them by name, instead of wildcard. But why does just one of them (and it doesn't matter which one) make it work? Remember it only happens for the modules that got moved between Qt5 and 6, the difference between how we treat them is that we import those ones by name. So maybe it worries that the named variables would be overwritten by later wildcard imports? Yes indeed, commenting our all named imports from PySide2, but leaving in the wildcards, make it work. Interesting. More evidence that it treats the named import differently, if out of all four bindings you only have PyQt6 installed it works fine.

Does changing the order of the imports in the machinery file make it work as well? (I think it does lol.) But even that would be weird because with the last one uninstalled it can still be made to work.

It seems it selects the last imported definition and if an object is imported by name in any of them then then if any of the later imports may have overridden the name but the imports aren't available it'll give up. Uh, not quite, if pyside2 is installed but not pyside6 it works fine...

Okay I think it is like this:

# pip3 install PyQt5-stubs
from PyQt5.QtWebEngineWidgets import QWebEngineHistory  # defined
from PyQt6.QtWebEngineCore import *  # no impact
from PySide2.QtWebEngineWidgets import QWebEngineHistory  # oh you assigned something undefined to it? Undefined!
from PySide6.QtWebEngineCore import *  # no impact
# (x) no autocomplete for QWebHistory

# pip3 install PyQt5-stubs PySide6
from PyQt5.QtWebEngineWidgets import QWebEngineHistory  # defined
from PyQt6.QtWebEngineCore import *  # no impact
from PySide2.QtWebEngineWidgets import QWebEngineHistory  # oh you assigned something undefined to it? Undefined!
from PySide6.QtWebEngineCore import *  # redefined
# (/) yes autocomplete for QWebEngineHistory (with PySide6)

So removing PySide2 "support" should fix this. BUT, then if you have PySide6 installed you would get autocompletion for that, and since we don't actually support it that is probably not so good.

[toofar]

I raised an issue with pyright here, lets see what they say about working in the same way as mypy: microsoft/pyright#4060

It seems that you can't tell it machinery.USE_PYQT6 is a constant but USE_PYQT6 works fine. So if we changed the relevant imports to be like from qutebrowser.qt.machinery import USE_PYQT6, ... that should work and be better than carefully juggling import order like I did in 4cf6aa8

Edit: pyright has already pushed a release, 1.1.276, to let us use module attributes as constants.

[The-Compiler]

I think this is partially done - the remaining part being getting tox -e mypy-pyqt6 to pass (possibly at the expense of breaking mypy-pyqt5).

[The-Compiler]

Both mypy-pyqt6 and mypy-pyqt5 work now, and I adjusted the pyrightconfig.json to assume Qt 6.