-
Notifications
You must be signed in to change notification settings - Fork 21
PySide6: make Signal and SignalInstance generic and populate argument types #41
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
base: master
Are you sure you want to change the base?
Conversation
This change allows the type and number of arguments of connected functions to be validated.
|
This looks like an exciting addition to the types! It'll be useful to be able to validate our signals and slots in this way. Because class MainDialog(QtWidgets.QDialog):
signal1 = QtCore.Signal() # Need type annotation for "signal1" [var-annotated]
signal2 = QtCore.Signal(int) # Need type annotation for "signal2" [var-annotated]
signal3 = QtCore.Signal(int, str) # Need type annotation for "signal3" [var-annotated]
signal4 = QtCore.Signal((int,), (str,)) # Need type annotation for "signal4" [var-annotated]
signal5 = QtCore.Signal((int, int), (str, str)) # Need type annotation for "signal5" [var-annotated]
signal6 = QtCore.Signal((int,), (int, int)) # Need type annotation for "signal6" [var-annotated]A user needs to correct this with the following: signal1: QtCore.Signal[()] = QtCore.Signal()
signal2: QtCore.Signal[int] = QtCore.Signal(int)
signal3: QtCore.Signal[int, str] = QtCore.Signal(int, str)
signal4: QtCore.Signal[int] = QtCore.Signal((int,), (str,))
signal5: QtCore.Signal[int, int] = QtCore.Signal((int, int), (str, str))
signal6: QtCore.Signal[int] = QtCore.Signal((int,), (int, int))In the above example, mypy is unhappy with the signals that have multiple signatures: It's possible to bind type variables based on the arguments to the constructor, but we have an additional layer of complexity here were we're passing We can remove the need to type annotate a plain Signal at least (so class Signal(typing.Generic[*_SignalTypes]):
@typing.overload
def __init__(self: Signal[()], /, name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self, /, *types: type, name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self, /, types: tuple[type, ...], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...I've seen type stubs where a limited set of overloads are supplied to allow for a limited number of arguments to be typed appropriately (EDIT: this is also documented as sometimes being necessary here: https://typing.python.org/en/latest/spec/generics.html#overloads-for-accessing-individual-types). For example, the following would allow up to three arguments to by fully type checked, and we could add more depending on what we deep to be a "normal" number of arguments: class Signal(typing.Generic[*_SignalTypes]):
@typing.overload
def __init__(self: Signal[()], /, name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[T1], /, arg1: type[T1], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[T1, T2], /, arg1: type[T1], arg2: type[T2], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[T1, T2, T3], /, arg1: type[T1], arg2: type[T2], arg3: type[T3], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self, /, *types: type, name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[T1], /, types: tuple[type[T1]], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[T1, T2], /, types: tuple[type[T1], type[T2]], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[T1, T2, T3], /, types: tuple[type[T1], type[T2], type[T3]], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self, /, types: tuple[type, ...], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...This suggested change doesn't completely account for Signals with multiple signatures, because it incorrectly flags issues when you try to connect a slot with any signature other than the signal's default signature. |
|
I can see it being useful to bind the class Signal(typing.Generic[*_SignalTypes]):
@typing.overload
def __init__(self: Signal[tuple[()]], /, name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[tuple[T1]], /, arg1: type[T1], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[tuple[T1, T2]], /, arg1: type[T1], arg2: type[T2], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[tuple[T1, T2, T3]], /, arg1: type[T1], arg2: type[T2], arg3: type[T3], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self, /, *types: type, name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[tuple[T1]], /, types: tuple[type[T1]], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[tuple[T1, T2]], /, types: tuple[type[T1], type[T2]], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self: Signal[tuple[T1, T2, T3]], /, types: tuple[type[T1], type[T2], type[T3]], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...
@typing.overload
def __init__(self, /, types: tuple[type, ...], *types2: tuple[type, ...], name: str | None = ..., arguments: Optional[List[str]] = ...) -> None: ...I haven't though about how to turn a Signal annotated in this way into a |
|
On the off chance this it's useful for testing, here's what I've been using to see what the type checker will output. import sys
from PySide6 import QtCore, QtWidgets
class MainDialog(QtWidgets.QDialog):
signal1 = QtCore.Signal()
signal2 = QtCore.Signal(int)
signal3 = QtCore.Signal(int, str)
signal4 = QtCore.Signal((int,), (str,))
signal5 = QtCore.Signal((int, int), (str, str))
signal6 = QtCore.Signal((int,), (int, int))
def __init__(self) -> None:
super().__init__()
self._connect_signals()
main_layout = QtWidgets.QVBoxLayout()
layout = QtWidgets.QGridLayout()
button = QtWidgets.QPushButton("()")
button.pressed.connect(self._emitSignal1)
layout.addWidget(button, 0, 0)
button = QtWidgets.QPushButton("(int)")
button.clicked.connect(self._emitSignal2)
layout.addWidget(button, 0, 1)
button = QtWidgets.QPushButton("(int, str)")
button.clicked.connect(self._emitSignal3)
layout.addWidget(button, 0, 2)
button = QtWidgets.QPushButton("((int,), (str,))")
button.clicked.connect(self._emitSignal4)
layout.addWidget(button, 1, 0)
button = QtWidgets.QPushButton("((int, int), (str, str))")
button.clicked.connect(self._emitSignal5)
layout.addWidget(button, 1, 1)
button = QtWidgets.QPushButton("((int,), (int, int))")
button.clicked.connect(self._emitSignal6)
layout.addWidget(button, 1, 2)
main_layout.addLayout(layout)
self._text_edit = QtWidgets.QPlainTextEdit()
main_layout.addWidget(self._text_edit)
button_box = QtWidgets.QDialogButtonBox(
QtWidgets.QDialogButtonBox.StandardButton.Ok
| QtWidgets.QDialogButtonBox.StandardButton.Cancel
)
button_box.accepted.connect(self.accept)
button_box.rejected.connect(self.reject)
main_layout.addWidget(button_box)
self.setLayout(main_layout)
def _connect_signals(self) -> None:
self.signal1.connect(self.slot1)
# All remaining slots require an argument
self.signal1.connect(self.slot2a)
self.signal1.connect(self.slot2b)
self.signal1.connect(self.slot2c)
self.signal1.connect(self.slot3)
self.signal1.connect(self.slot4)
self.signal2.connect(self.slot1)
self.signal2.connect(self.slot2a)
# Argument has the wrong type,
# but Qt happens to be able to coerce an int to str.
self.signal2.connect(self.slot2b)
self.signal2.connect(self.slot2c)
# Is missing a required argument
self.signal2.connect(self.slot3)
self.signal2.connect(self.slot4)
self.signal3.connect(self.slot1)
self.signal3.connect(self.slot2a)
self.signal3.connect(self.slot2b)
self.signal3.connect(self.slot2c)
self.signal3.connect(self.slot3)
self.signal3.connect(self.slot4)
self.signal4.connect(self.slot1)
self.signal4.connect(self.slot2a)
# Argument 1 has the wrong type,
# but Qt happens to be able to coerce an int to str.
self.signal4.connect(self.slot2b)
self.signal4.connect(self.slot2c)
# Is missing a required argument
self.signal4.connect(self.slot3)
self.signal4.connect(self.slot4)
self.signal5.connect(self.slot1)
# Argument 1 might have the wrong type
# depending on the Signal signature.
self.signal5.connect(self.slot2a)
# Argument 1 might have the wrong type
# depending on the Signal signature.
self.signal5.connect(self.slot2b)
self.signal5.connect(self.slot2c)
self.signal5.connect(self.slot3)
self.signal5.connect(self.slot4)
self.signal6.connect(self.slot1)
self.signal6.connect(self.slot2a)
# Argument 1 has the wrong type,
# but Qt happens to be able to coerce an int to str.
self.signal6.connect(self.slot2b)
self.signal6.connect(self.slot2c)
self.signal6.connect(self.slot3)
self.signal6.connect(self.slot4)
@QtCore.Slot()
def _emitSignal1(self) -> None:
self._text_edit.clear()
self.signal1.emit()
@QtCore.Slot()
def _emitSignal2(self) -> None:
self._text_edit.clear()
self.signal2.emit(1)
@QtCore.Slot()
def _emitSignal3(self) -> None:
self._text_edit.clear()
self.signal3.emit(1, "one")
@QtCore.Slot()
def _emitSignal4(self) -> None:
self._text_edit.clear()
self.signal4.emit(1)
self.signal4[int].emit(2)
self.signal4[str].emit("one")
@QtCore.Slot()
def _emitSignal5(self) -> None:
self._text_edit.clear()
self.signal5.emit(1, 2)
self.signal5[int, int].emit(3, 4)
self.signal5[str, str].emit("one", "two")
@QtCore.Slot()
def _emitSignal6(self) -> None:
self._text_edit.clear()
self.signal6.emit(1)
self.signal6[int].emit(2)
self.signal6[int, int].emit(3, 4)
@QtCore.Slot()
def slot1(self) -> None:
self._text_edit.insertPlainText("slot1: ()\n")
@QtCore.Slot()
def slot2a(self, arg1: int) -> None:
self._text_edit.insertPlainText(f"slot2a: ({arg1!r})\n")
@QtCore.Slot()
def slot2b(self, arg1: str) -> None:
self._text_edit.insertPlainText(f"slot2b: ({arg1!r})\n")
@QtCore.Slot(int)
@QtCore.Slot(str)
def slot2c(self, arg1: int | str) -> None:
self._text_edit.insertPlainText(f"slot2c: ({arg1!r})\n")
@QtCore.Slot()
def slot3(self, arg1: int | str, arg2: int | str) -> None:
self._text_edit.insertPlainText(f"slot3: ({arg1!r}, {arg2!r})\n")
@QtCore.Slot()
def slot4(self, arg1: int | str, arg2: int | str | None = None) -> None:
self._text_edit.insertPlainText(f"slot4: ({arg1!r}, {arg2!r})\n")
if __name__ == "__main__":
app = QtWidgets.QApplication(sys.argv)
window = MainDialog()
sys.exit(window.exec()) |
This PR adds type safety to signals in a few ways:
SignalInstance.connectenforces the number and types of arguments for passed callables.SignalInstance.emitenforces the number and types of arguments providedSignal instances on native classes are populated with the requisite types in the stubs, and custom signals can also be properly typed to add enforcement.
It's important to note that the actual
SignalandSignalInstanceclasses are not generic, so it is not safe to index into them to provide type variables at runtime, so you'll need to use one of the following workarounds:from __future__ import annotationsto defer evaluation of typesSignalandSignalInstanceto add__class_getitem__