python-qt-tools / PyQt5-stubs

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

QApplication.instance() should return undocumented QApplication() instead of QCoreApplication #141

Closed bluebird75 closed 2 years ago

bluebird75 commented 3 years ago

Hi,

According to Qt documentation QApplication.instance() is an inherited static method from QCoreApplication.instance() which returns a QCoreApplication.

However, in practice, it does return a QApplication wrapped object . But using QApplication specific methods (like palette() in my case) generates a typing error because this is not a QCoreApplication method.

If you agree with the issue, I'll submit a PR to fix it.

Example code to show the problem :

Python 3.8.8 (tags/v3.8.8:024d805, Feb 19 2021, 13:08:11) [MSC v.1928 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from PyQt5.QtWidgets import QApplication
>>> app = QApplication([])
>>> QApplication.instance()
<PyQt5.QtWidgets.QApplication object at 0x014DD3D0>
altendky commented 3 years ago
Python 3.8.6 (default, Oct  8 2020, 19:34:05) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from PyQt5.QtCore import QCoreApplication
>>> from PyQt5.QtWidgets import QApplication
>>> app = QCoreApplication([])
>>> QApplication.instance()
<PyQt5.QtCore.QCoreApplication object at 0x7ff7e5f05d30>

Others here may have different opinions, but hinting return of a QCoreApplication does seem correct since what is returned is an instance of QCoreApplication. Either directly or by way of being an instance of a subclass. I'll admit I'm a bit of a stickler for 'correct' here, at least when I can figure out what correct is. I also generally dislike global singletons, but sure, they are a reality here. Presuming you don't want to pass the application to where it is needed, perhaps creating your own global that is hinted as you wish would allow the hassle of handling this to be isolated to one place. This takes care of the Union with None bit as well. I'll admit that I don't really like the fact that application is hinted for awhile before it exists, and that could be avoided by just creating the application globally... which also is ugly... but I think there's just some ugly that is going to exist and the only question is where it is put.

None of these are super nice. :[ Is the solution below a decent compromise? Remember that even if the hint was switched from QCoreApplication to QApplication, there would still be the None possibility in your way. That bit was discussed over in https://github.com/python-qt-tools/PyQt5-stubs/pull/126.

from PyQt5 import QtWidgets
import typing

application: QtWidgets.QApplication

def other() -> None:
    if typing.TYPE_CHECKING:
        reveal_type(application)
        reveal_type(QtWidgets.QApplication.instance())
    print(application)

def main() -> None:
    global application
    application = QtWidgets.QApplication([])
    other()

main()
$ venv/bin/python x.py
<PyQt5.QtWidgets.QApplication object at 0x7fd079455940>
$ venv/bin/mypy x.py
x.py:10: note: Revealed type is 'PyQt5.QtWidgets.QApplication'
x.py:11: note: Revealed type is 'Union[PyQt5.QtCore.QCoreApplication, None]'
bluebird75 commented 2 years ago

It seems strange to me that mypy let you assign a QCoreApplication instance to a QApplication typed variable.

But the idea that stubs should reflect reality stands. We are not here to change Qt's design, so closing it.