python-qt-tools / PyQt5-stubs

Stubs for PyQt5
GNU General Public License v3.0
67 stars 31 forks source link

QMessageBox expects "StandardButtons" and fails #108

Closed pmiddend closed 3 years ago

pmiddend commented 3 years ago

This code seems to fail with the stubs:

QMessageBox.question(self, "Confirm", "Are you sure?", QMessageBox.Yes | QMessageBox.No)

Tells me:

Argument 4 to "question" of "QMessageBox" has incompatible type "int"; expected "Union[StandardButtons, StandardButton]"

But the code should work, should it not?

altendky commented 3 years ago

Thanks for the ticket. I forget the details but I believe I ran into this myself a few weeks back and just ignored it because there was so much else to do on the stubs. They are a lot better now so we will try to take a look at this for making a real fix. It might just be to expand that Union to include int, maybe. Or perhaps the .__or__() being missing is the issue. Anyways, we'll have to dig around. The enumerations are definitely one of the pain points right now.

pmiddend commented 3 years ago

Thanks for the quick reply. It's no big one for me as well, just something I noticed and type: ignored for now.

BryceBeagle commented 3 years ago

Some observations

>>> QMessageBox.Ok
1024
>>> QMessageBox.StandardButton.Ok
1024

>>> type(QMessageBox.Ok)
<class 'PyQt5.QtWidgets.QMessageBox.StandardButton'>
>>> type(QMessageBox.StandardButton.Ok)
<class 'PyQt5.QtWidgets.QMessageBox.StandardButton'>

>>> isinstance(QMessageBox.Ok, QMessageBox.StandardButton)
True

>>> QMessageBox.Ok is QMessageBox.StandardButton.Ok
False
>>> QMessageBox.Ok == QMessageBox.StandardButton.Ok
True

For some reason mypy thinks that QMessageBox.Ok is an int, when it's not. Although it does print as one

altendky commented 3 years ago

The enumerations aren't implementing .__or__() so StandardButton gets int.__or__() which is hinted to yield an int and StandardButtons gets nothing since int/object don't provide .__or__(). class I is just a trivial example showing what happens when you inherit from int and don't implement .__or__().

from PyQt5 import QtWidgets

reveal_type(QtWidgets.QMessageBox.NoButton)
reveal_type(QtWidgets.QMessageBox.StandardButton.NoButton)
reveal_type(QtWidgets.QMessageBox.StandardButtons.NoButton)

reveal_type(QtWidgets.QMessageBox.NoButton | QtWidgets.QMessageBox.NoButton)

reveal_type(QtWidgets.QMessageBox.StandardButton.__or__)
reveal_type(QtWidgets.QMessageBox.StandardButtons.__or__)

class I(int): ...

reveal_type(I() | I())
x.py:4: note: Revealed type is 'PyQt5.QtWidgets.QMessageBox.StandardButton'
x.py:5: error: "Type[StandardButton]" has no attribute "NoButton"
x.py:5: note: Revealed type is 'Any'
x.py:6: error: "Type[StandardButtons]" has no attribute "NoButton"
x.py:6: note: Revealed type is 'Any'
x.py:8: note: Revealed type is 'builtins.int'
x.py:10: note: Revealed type is 'def (builtins.int, builtins.int) -> builtins.int'
x.py:11: error: Unsupported left operand type for | ("Type[StandardButtons]")
x.py:11: note: Revealed type is 'Any'
x.py:15: note: Revealed type is 'builtins.int'
Found 3 errors in 1 file (checked 1 source file)

(I need to find a way to have mypy interlace these as comments or something because this is really a pain to try to keep track of the output vs. the input)

https://github.com/stlehmann/PyQt5-stubs/blob/811462b34ee151b898289ae8f1de8af30c690c55/PyQt5-stubs/QtWidgets.pyi#L2749

https://github.com/stlehmann/PyQt5-stubs/blob/811462b34ee151b898289ae8f1de8af30c690c55/PyQt5-stubs/QtWidgets.pyi#L2789-L2802

https://github.com/stlehmann/PyQt5-stubs/blob/811462b34ee151b898289ae8f1de8af30c690c55/PyQt5-stubs/sip.pyi#L34

altendky commented 3 years ago

There's something to look at I guess. mypy doesn't like the overlap and I haven't dug into how to deal with that. Not sure if we want to dig into the other operators at the moment or not.

altendky commented 3 years ago

Do we consider this fixed by #109? That even got released in 5.15.2.0.

BryceBeagle commented 3 years ago

Looks like it. The original issue was about

QMessageBox.Yes | QMessageBox.No

which should work now.

altendky commented 3 years ago

@pmiddend, I'll go ahead and close this. If it is not actually working for you then please do reopen this and explain (with code and output :]). And if there's another problem, certainly open up another issue. Thanks for the issue report.

Cheers, -kyle