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

fix pyqtSlot result parameter type and Callable generic #102

Merged
merged 10 commits into from
Oct 7, 2020

Conversation

altendky
Copy link
Collaborator

@altendky altendky commented Oct 4, 2020

No description provided.

@altendky altendky mentioned this pull request Oct 4, 2020
3 tasks
PyQt5-stubs/QtCore.pyi Outdated Show resolved Hide resolved
tests/pyqtslot.py Show resolved Hide resolved
tests/pyqtslot.py Show resolved Hide resolved
PyQt5-stubs/QtCore.pyi Outdated Show resolved Hide resolved
@@ -9264,7 +9264,7 @@ def oct_(s: QTextStream) -> QTextStream: ...
def bin_(s: QTextStream) -> QTextStream: ...
def Q_RETURN_ARG(type: typing.Any) -> QGenericReturnArgument: ...
def Q_ARG(type: typing.Any, data: typing.Any) -> QGenericArgument: ...
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Optional[str] = ...) -> typing.Callable[[FuncT], FuncT]: ...
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[object] = ...) -> typing.Callable[[FuncT], FuncT]: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about the typing.Type[object]. I starting with Type because I was trying to relate it to the function return type but... I didn't get that working. Maybe this should just be object/Any?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could FuncT be made generic? I think this would work

T = TypeVar("T")
FuncT = typing.TypeVar('FuncT', bound=typing.Callable[..., T])
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[T] = ...) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...

However, looking at the docs:

result – the type of the result and may be a Python type object or a string that specifies a C++ type

Not sure how the C++ string would interact with this. Maybe

def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Union[typing.Type[T], str] = ...) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that types has the same comment in the docs about "may be a Python type object or a string", so maybe we can be more specific than typing.Any

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean "can't be more specific"? When I read this I thought you made a comment then backed away from it. I did make an effort to relate the result parameter to the return type of the typing.Callable parameter and return but failed. I could make another attempt for you to look at.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No those were two different comments. I was suggesting that we change result to use a generic. Then I noticed that types is documented in a similar fashion and remarked that we could maybe do better than its current typing.Any

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the types, they can be any types including user defined and they don't necessarily even inherit from type what with metatypes, right? (plus str) So the only thing I can think of is to try to relate it with the parameters of the wrapped callable, but mypy doesn't do that yet. There's some related PEP but I can't find it at the moment. Do you have something specific in mind here?

I'll try what you shared though it looks familiar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're probably right that types is as good as mypy currently allows

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's yours along with the existing and a Protocol attempt.

https://mypy-play.net/?mypy=latest&python=3.8&gist=314620351fccbbfd297833d345b40a6d

import typing


FuncT = typing.TypeVar('FuncT', bound=typing.Callable[..., typing.Any])

def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Any = ..., revision: int = ...) -> typing.Callable[[FuncT], FuncT]: ...


T = typing.TypeVar("T")
FuncT2 = typing.TypeVar('FuncT2', bound=typing.Callable[..., T])
def pyqtSlot2(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[T] = ...) -> typing.Callable[[FuncT2[T]], FuncT2[T]]: ...


T_covariant = typing.TypeVar("T_covariant", covariant=True)
class P(typing.Protocol, typing.Generic[T_covariant]):
    def __call__(self, *args: typing.Any, **kwargs: typing.Any) -> T_covariant: ...


def pyqtSlot3(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[T_covariant] = ...) -> typing.Callable[[P[T_covariant]], P[T_covariant]]: ...


def f(s: str, i: int) -> float: ...


reveal_type(pyqtSlot(result=float)(f))
reveal_type(pyqtSlot2(result=float)(f))
reveal_type(pyqtSlot3(result=float)(f))
main.py:11: error: Type variable "FuncT2" used with arguments
main.py:25: note: Revealed type is 'def (s: builtins.str, i: builtins.int) -> builtins.float'
main.py:26: error: Value of type variable "FuncT2" of function cannot be "Callable[[str, int], float]"
main.py:26: note: Revealed type is 'def (s: builtins.str, i: builtins.int) -> builtins.float'
main.py:27: note: Revealed type is 'main.P[builtins.float*]'
main.py:27: error: Argument 1 has incompatible type "Callable[[str, int], float]"; expected "P[float]"
Found 3 errors in 1 file (checked 1 source file)

I don't think we get to make a generic typevar.

Copy link
Collaborator

@BryceBeagle BryceBeagle Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we need to make FuncT a TypeVar?

import typing

S = typing.TypeVar("S")
T = typing.TypeVar("T")
FuncT = typing.Callable[..., S]

def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[T] = ...) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...


def f(s: str, i: int) -> float: ...


reveal_type(pyqtSlot(result=float)(f))
main.py:13: note: Revealed type is 'def (*Any, **Any) -> builtins.float*'
def f(s: str, i: int) -> float: ...


reveal_type(pyqtSlot(result=bool)(f))
main.py:13: note: Revealed type is 'def (*Any, **Any) -> builtins.bool*'
main.py:13: error: Argument 1 has incompatible type "Callable[[str, int], float]"; expected "Callable[..., bool]"
Found 1 error in 1 file (checked 1 source file)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You win (I think), thanks.

PyQt5-stubs/QtCore.pyi Outdated Show resolved Hide resolved
@@ -9264,7 +9264,7 @@ def oct_(s: QTextStream) -> QTextStream: ...
def bin_(s: QTextStream) -> QTextStream: ...
def Q_RETURN_ARG(type: typing.Any) -> QGenericReturnArgument: ...
def Q_ARG(type: typing.Any, data: typing.Any) -> QGenericArgument: ...
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Optional[str] = ...) -> typing.Callable[[FuncT], FuncT]: ...
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[object] = ...) -> typing.Callable[[FuncT], FuncT]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could FuncT be made generic? I think this would work

T = TypeVar("T")
FuncT = typing.TypeVar('FuncT', bound=typing.Callable[..., T])
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[T] = ...) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...

However, looking at the docs:

result – the type of the result and may be a Python type object or a string that specifies a C++ type

Not sure how the C++ string would interact with this. Maybe

def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Union[typing.Type[T], str] = ...) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...

@@ -9264,7 +9264,7 @@ def oct_(s: QTextStream) -> QTextStream: ...
def bin_(s: QTextStream) -> QTextStream: ...
def Q_RETURN_ARG(type: typing.Any) -> QGenericReturnArgument: ...
def Q_ARG(type: typing.Any, data: typing.Any) -> QGenericArgument: ...
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Optional[str] = ...) -> typing.Callable[[FuncT], FuncT]: ...
def pyqtSlot(*types: typing.Any, name: typing.Optional[str] = ..., result: typing.Type[object] = ...) -> typing.Callable[[FuncT], FuncT]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that types has the same comment in the docs about "may be a Python type object or a string", so maybe we can be more specific than typing.Any

@altendky
Copy link
Collaborator Author

altendky commented Oct 6, 2020

Do you think we should try to do more here now?

@BryceBeagle
Copy link
Collaborator

Did you see my comment about making FuncT generic?

@BryceBeagle
Copy link
Collaborator

BryceBeagle commented Oct 7, 2020

Sorry not trying to drag this PR on and on again, but I think we need this for the overloads:

@typing.overload
def pyqtSlot(*types: typing.Any) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, result: typing.Union[typing.Type[T], str]) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str, result: typing.Union[typing.Type[T], str]) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str, revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, result: typing.Union[typing.Type[T], str], revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str, result: typing.Union[typing.Type[T], str], revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...

What I changed:

  • Added overloads for each combination of all three optional arguments. Note that they're not optional... just overloads *. This is pretty verbose, but I don't know of a better option.
  • Removed the Optional-ality of the str arguments
  • Removed ... defaults from arguments because they don't actually have defaults
  • Combined the result types into a Union because the Union-ization doesn't affect the return type/other arguments

* Note: This contradicts the documentation here where the later arguments are in ever-nesting brackets. The documentation implies that you can't use revision without using result but this doesn't appear to be the case. Some of the examples in the docs even contradict the specification.

>>> pyqtSlot(int, name=None)
TypeError: pyqtSlot() argument 1 must be str, not None

Side note: It looks like argument 1 here is a bugged miscount on PyQt's side, as this is argument 2.

This is further contradicted pyqtSlot.__doc__:

@pyqtSlot(*types, name: Optional[str], result: Optional[str])

This is a decorator applied to Python methods of a QObject that marks them
as Qt slots.
The non-keyword arguments are the types of the slot arguments and each may
be a Python type object or a string specifying a C++ type.
name is the name of the slot and defaults to the name of the method.
result is type of the value returned by the slot.

We should probably mention this to Phil too.

This is a mess 🤦

@altendky
Copy link
Collaborator Author

altendky commented Oct 7, 2020

You can't use Union there with --strict because it makes the TypeVar be <nothing>. I'll expand and commit.

https://mypy-play.net/?mypy=latest&python=3.8&flags=strict&gist=9608229557029541b9a90553bce77414

import typing


T = typing.TypeVar("T")
FuncT = typing.Callable[..., T]
@typing.overload
def pyqtSlot(*types: typing.Any) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, result: typing.Union[typing.Type[T], str]) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str, result: typing.Union[typing.Type[T], str]) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str, revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, result: typing.Union[typing.Type[T], str], revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...
@typing.overload
def pyqtSlot(*types: typing.Any, name: str, result: typing.Union[typing.Type[T], str], revision: int) -> typing.Callable[[FuncT[T]], FuncT[T]]: ...

@pyqtSlot(str, result='int')
def func_str(s: str) -> int:
    return 42
main.py:6: error: An overloaded function outside a stub file must have an implementation
main.py:23: error: Argument 1 has incompatible type "Callable[[str], int]"; expected "Callable[..., <nothing>]"
Found 2 errors in 1 file (checked 1 source file)

Copy link
Collaborator

@BryceBeagle BryceBeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we probably also want to update the changelog too

@BryceBeagle
Copy link
Collaborator

fix `pyqtSlot` parameter typing and overloads

CHANGELOG.md Outdated Show resolved Hide resolved
@altendky altendky merged commit 70238e1 into python-qt-tools:master Oct 7, 2020
@altendky altendky deleted the pyqtslot_fixup branch October 7, 2020 22:49
bluebird75 pushed a commit to bluebird75/PyQt5-stubs that referenced this pull request Aug 24, 2021
fix `pyqtSlot` `result` parameter type and `Callable` generic
bluebird75 added a commit to bluebird75/PyQt5-stubs that referenced this pull request Apr 26, 2022
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

Successfully merging this pull request may close these issues.

2 participants