python-qt-tools / PyQt5-stubs

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

Move ignored stubtests errors from to_review to stubtest.allowlist #194

Closed bluebird75 closed 2 years ago

bluebird75 commented 2 years ago

Can we move these stubtests errors from to_review to officially ignored ?

What else is needed for the transition to happen ?

altendky commented 2 years ago

Have you reviewed them to make sure there is not a plausible solution to them other than ignoring? I don't recall at this point what the complaints were about these.

bluebird75 commented 2 years ago

stubtest complains about them but I could find a good reason why last time I looked. I think they were discovered with a more recent version of mypy/stubtest. I tried to debug stubtest to understand what is the problem for him but it is currently above my level of understanding.

Do you know why/how the previous lines in stubtest.allowlist were added ? The situation looks similar here. I need to check what's wrong with PyQt5.QtQml.QQmlListProperty but as far as I remember, there is nothing wrong here. It feels like a stubtest limitation/bug to me.

The-Compiler commented 2 years ago

I agree we should really look at those errors in detail instead of just dismissing them. From a very quick look, for most of those, stubtest seems to complain that:

error: PyQt5.QtCore.QCalendar.System.__new__ is inconsistent, stub argument "__x" differs from runtime argument "value"
error: PyQt5.QtCore.QCalendar.System.__new__ is inconsistent, stub argument "__x" has a default value but runtime argument does not
error: PyQt5.QtCore.QCalendar.System.__new__ is inconsistent, stub argument "__x" should be positional or keyword (remove leading double underscore)
error: PyQt5.QtCore.QCalendar.System.__new__ is inconsistent, runtime does not have argument "base"

and it indeed is correct! QtCore.QCalendar.System is an IntEnum at runtime:

>>> QCalendar.System.__mro__
(<enum 'System'>, <enum 'IntEnum'>, <class 'int'>, <enum 'Enum'>, <class 'object'>)

so we can do e.g.

>>> QCalendar.System(value=0)
<System.Gregorian: 0>

and of course this makes no sense:

>>> QCalendar.System(0, base=16)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: EnumMeta.__call__() got an unexpected keyword argument 'base'

but the stubs wrongly declares them as int instead of IntEnum:

https://github.com/python-qt-tools/PyQt5-stubs/blob/5cfd4436ab46469ed32036e4f50dfb6ce2c8a68f/PyQt5-stubs/QtCore.pyi#L4197

So at least for all those __new__ on enums, we should really fix the stubs instead.


For the PyQt5.QtQml.QQmlListProperty one, it complains:

error: PyQt5.QtQml.QQmlListProperty is not present in stub
Stub:
MISSING
Runtime:
QQmlListProperty<QObject>

and indeed that works (it just seems a string essentially?):

>>> from PyQt5.QtQml import QQmlListProperty
>>> QQmlListProperty
'QQmlListProperty<QObject>'
>>> type(QQmlListProperty)
<class 'PyQt5.QtQml.QQmlListProperty'>
>>> type(QQmlListProperty).__mro__
(<class 'PyQt5.QtQml.QQmlListProperty'>, <class 'str'>, <class 'object'>)

I have no idea why it's there and how it's used (don't know much about QML) - but again, it seems like a real issue with the stubs, so why should we ignore it?

The-Compiler commented 2 years ago

As for the existing ignores, I also took a quick look at them, and believe that those indeed should be ignored, because those are a stubtest/PyQt5 limitation. It complains that e.g.:

error: PyQt5.Qt3DRender.QBufferDataGenerator.__call__ is inconsistent, stub does not have *args argument "args"
Stub: at line 813
def (self: PyQt5.Qt3DRender.QBufferDataGenerator) -> PyQt5.QtCore.QByteArray
Runtime:
def (self, /, *args, **kwargs)

but the stubs are correct here! While help(QRandomGenerator) shows:

 |  __call__(self, /, *args, **kwargs)
 |      Call self as a function.

it doesn't actually take arbitrary *args / **kwargs:

>>> gen = QRandomGenerator()
>>> gen("test")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: QRandomGenerator.__call__(): too many arguments

so this seems to be caused by an issue in sip (I think) not correctly preserving those signatures for C++'s operator(). The other ignores for __init__ methods are very similar too, e.g.:

error: PyQt5.QtCore.QMimeData.__init__ is inconsistent, stub does not have *args argument "args"
Stub: at line 9429
def (self: PyQt5.QtCore.QMimeData)
Runtime:
def (self, /, *args, **kwargs)

and again, the stubs are correct:

>>> QMimeData("test")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: QMimeData(): too many arguments

There are a few remaining ones:

error: PyQt5.QtCore.pyqtBoundSignal.__call__ is not present in stub
Stub:
MISSING
Runtime:
<slot wrapper '__call__' of 'PyQt5.QtCore.pyqtBoundSignal' objects>

error: PyQt5.QtCore.pyqtSignal.__call__ is not present in stub
Stub:
MISSING
Runtime:
<slot wrapper '__call__' of 'PyQt5.QtCore.pyqtSignal' objects>

and calling (unbound) signals seems to work at runtime somehow, but doesn't do anything terribly useful. I suppose it's a PyQt5 implementation detail of some sorts:

>>> from PyQt5.QtCore import pyqtSignal, pyqtSlot
>>> pyqtSignal()
<unbound PYQT_SIGNAL )>
>>> pyqtSignal()()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: native Qt signal is not callable

finally, we have:

error: PyQt5.sip.array is not present at runtime
Stub: at line 42
<TypeInfo PyQt5.sip.array>
Runtime:
MISSING

which is debatable: sip.array really does not exist. It's just something we seem to need in the stubs. This one might need some closer investigation perhaps.

bluebird75 commented 2 years ago

Thanks for getting through this. The error message were really cryptic to me.

I'll address the others per your recommendations.

bluebird75 commented 2 years ago

This is now ready to be merged.

bluebird75 commented 2 years ago

Suggestion applied. Is this ready to be merged according to you ?

bluebird75 commented 2 years ago

Thanks for the timely review!