python-qt-tools / PyQt5-stubs

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

Handling of operation on WindowType and WindowFlags #142

Closed bluebird75 closed 2 years ago

bluebird75 commented 3 years ago

Hi,

Thanks for the great work on PyQt5 stubs, it is a pleasure to use it.

In my code, I often disable the annoying help button of the window bar with the following code dialog.setWindowFlags(dialog.windowFlags() & ~Qt.WindowContextHelpButtonHint)

Mypy fails on it with : src\mg_dialog_run_git_cmd.py:29: error: Unsupported operand types for & ("WindowFlags" and "int")

I am trying to enhance type checking for this case. There are actually two issues :

I have "implemented" the solution by annotating additional operations for both types. Is it something worth adding to the official stubs ?

altendky commented 3 years ago

Sounds a lot like https://github.com/python-qt-tools/PyQt5-stubs/issues/108. Not as in a duplicate, but as in the same form of solution may help here. I'll try to look this over more later. Or, if you want to take a stab at it, I'd be happy to look over a PR. :]

bluebird75 commented 2 years ago

Fixed by #153

adam-grant-hendry commented 2 years ago

@bluebird75 Is there a reason why PyQt5-stubs used such a difficult implementation to incorporate WindowFlags

PyQt5-stubs/QtWidgets.pyi

    def setWindowFlags(self, type: typing.Union[QtCore.Qt.WindowFlags, QtCore.Qt.WindowType]) -> None: ...

PyQt5-stubs/QtCore.pyi

class WindowFlags(sip.simplewrapper):

        @typing.overload
        def __init__(self) -> None: ...
        @typing.overload
        def __init__(self, f: typing.Union['Qt.WindowFlags', 'Qt.WindowType']) -> None: ...
        @typing.overload
        def __init__(self, a0: 'Qt.WindowFlags') -> None: ...

        def __hash__(self) -> int: ...
        def __bool__(self) -> int: ...
        def __invert__(self) -> 'Qt.WindowFlags': ...
        def __index__(self) -> int: ...
        def __int__(self) -> int: ...

when PyQt6-stubs opted to exclude WindowFlags and leverage python's built-in enum.IntFlags:

PyQt6-stubs/QtWidgets.pyi

 def setWindowFlags(self, type: QtCore.Qt.WindowType) -> None: ...

PyQt6-stubs/QtCore.pyi

class WindowType(enum.IntFlag):
    Widget = ...  # type: Qt.WindowType
    ...
adam-grant-hendry commented 2 years ago

For the record, I did nearly what PyQt6 did, but instead of ignoring WindowFlags entirely, I simply made it an alias to WindowType, so my stubs look like this:

PyQt6-stubs/QtWidgets.pyi

def setWindowFlags(self, type: QtCore.Qt.WindowFlags) -> None: ...

PyQt6-stubs/QtCore.pyi

WindowFlags = WindowType

class WindowType(enum.IntFlag):
    Widget = ...  # type: Qt.WindowType
    ...
The-Compiler commented 2 years ago

@adam-grant-hendry Because that's what PyQt5 and PyQt6 themselves do. PyQt5 enums are custom implementations, while PyQt6 enums use Python enums.

bluebird75 commented 2 years ago

Hi,

Thanks for the feedback.

First, you talk about "implementation" regarding PyQt5-stubs or PyQt6-stubs but the implementation is actually PyQt5 and PyQt6. The -stubs version are just a description of how the implementation behaves.

Why the current choice of describing this stubs with WindowType and WindowFlags and many operations ?

For me, the answer is : because we strive to be correct and precise

Your choice of simplifying the stubs is interesting. I do sympathize with the idea except that I have a hard time giving up preciseness. I am sure that I can find some cases where your simplification yields different results between mypy and the actual implementation.

Whether that's a real problem, I don't know but I like it the way I did it.

If you have other feedback, don't hesitate, it's very constructive.

bluebird75 commented 2 years ago

Actually, I tried for 10 minutes to prove your simplification wrong without success. So your proposal might be practically correct, even if not reflecting the actual implementation, and carrying a lie : WindowType and WindowFlags are 2 different classes.

adam-grant-hendry commented 2 years ago

Because that's what PyQt5 and PyQt6 themselves do. PyQt5 enums are custom implementations, while PyQt6 enums use Python enums.

Ah, I see now, thank you. I wasn't sure, so I was scratching my head when looking at the two. After downloading the source and grepping the sip files, I saw:

PyQt-5.15.6/sip/QtCore/qnamespace.sip

185     enum WindowType
198         WindowType_Mask,
241     typedef QFlags<Qt::WindowType> WindowFlags;
337         WA_X11NetWmWindowTypeDesktop,
338         WA_X11NetWmWindowTypeDock,
339         WA_X11NetWmWindowTypeToolBar,
340         WA_X11NetWmWindowTypeMenu,
341         WA_X11NetWmWindowTypeUtility,
342         WA_X11NetWmWindowTypeSplash,
343         WA_X11NetWmWindowTypeDialog,
344         WA_X11NetWmWindowTypeDropDownMenu,
345         WA_X11NetWmWindowTypePopupMenu,
346         WA_X11NetWmWindowTypeToolTip,
347         WA_X11NetWmWindowTypeNotification,
348         WA_X11NetWmWindowTypeCombo,
349         WA_X11NetWmWindowTypeDND,
1815 QFlags<Qt::WindowType> operator|(Qt::WindowType f1, QFlags<Qt::WindowType> f2);

PyQt6-6.3.1/sip/QtCore/qnamespace.sip

141     enum AlignmentFlag /BaseType=IntFlag/
161     enum TextFlag /BaseType=IntFlag/
183     enum WindowType /BaseType=IntFlag/
225     typedef QFlags<Qt::WindowType> WindowFlags;

Hence, indeed, WindowFlags are a WindowType in PyQt6.

So your proposal might be practically correct, even if not reflecting the actual implementation, and carrying a lie : WindowType and WindowFlags are 2 different classes.

w.r.t @The-Compiler 's explanation, to my surprise it appears it doesn't actually carry a lie (at least w.r.t. PyQt6, but perhaps not Qt6). Indeed, both are typed as WindowType, so whether WindowType is used explicitly in place of WindowFlags or WindowFlags are aliased for WindowType, the result is literally correct for PyQt6.

I appreciate your strive for preciseness; I prefer it and is why I asked the question about the difference in case I was doing something incorrect.

Thank you for all the excellent work! I love these stubs! Keep it up!

ASIDE: PyQt6-6.3.1 just got released yesterday! :tada:

The-Compiler commented 2 years ago

Actually, I tried for 10 minutes to prove your simplification wrong without success. So your proposal might be practically correct, even if not reflecting the actual implementation

For starters, this doesn't work, but would most likely be marked as valid by those "simplified" stubs:

>>> from PyQt5.QtCore import Qt
>>> Qt.WindowType.Widget.name
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'WindowType' object has no attribute 'name'

For the record, I did nearly what PyQt6 did, but instead of ignoring WindowFlags entirely, I simply made it an alias to WindowType, so my stubs look like this: [...]

Note that WindowFlags does not exist at all in PyQt6.

adam-grant-hendry commented 2 years ago

Note that WindowFlags does not exist at all in PyQt6.

I showed that it does in the sip as a typedef.

adam-grant-hendry commented 2 years ago

For starters, this doesn't work, but would most likely be marked as valid by those "simplified" stubs:

That is PyQt5. I'm only talking about PyQt6 and PySide6. After what you revealed about the differences between PyQt5 and PyQt6, it doesn't make sense to refactor PyQt5.

bluebird75 commented 2 years ago

typedef QFlags WindowFlags; Hence, indeed, WindowFlags are a WindowType in PyQt6.

Your statement is in contradiction with the code you showed above. WindowFlags is a QFlags type, while WindowType is a just an enum.

Anyway, I'm glad you are happy with the stubs. PyQt6 is on my radar but I am working on PySide2 stubs currently. Things will come in time.

Are you working on your own stubs ?