QtCore.QSocketNotifier socket param is different in PySide2

[Ahuge]

The QtCore.QSocketNotifier is supported to take an int representing the fileno of the socket as it's first param.

In all bindings except PySide2, it works.
In PySide2 it appears to call "fileno" on the first arg you pass it.

The PySide2 bug is here:

• PySide2 bug #PYSIDE-629

import socket

from Qt import QtCore,QtWidgets
# from PySide2 import QtCore,QtWidgets

app = QtWidgets.QApplication([])

w_sock, r_sock = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM)
notifier = QtCore.QSocketNotifier(r_sock.fileno(), QtCore.QSocketNotifier.Read)
# >>> AttributeError: 'int' object has no attribute 'fileno'

[Ahuge]

What do we think we should do here?

• Try to get it fixed in PySide2?
• Build something in QtCompat that conforms to broken PySide2?
• Build a wrapper for PySide2 that builds a socket object from the passed fileno and then pass the new socket to PySide2 to turn back into a fileno?
• Other?

[Ahuge]

for number 3, here is an example of how we would turn the fd back into a socket for PySide2

def _pyside2_qtcore_qsocketnotifier(socket, notif_type, parent=None):
    import socket
    socket_obj = socket.fromfd(socket, socket.AF_UNIX, socket.SOCK_STREAM)
    mod = getattr(getattr(Qt, "_pyside2"), "QtCore")
    klass = getattr(mod, "QSocketNotifier")
    return klass(socket_obj, notif_type, parent)

[MHendricks]

I was going to say number 2 would be easy to implement by adding a QtCompat namespace decorator using the code in _pyside2_qtcore_qsocketnotifier for PySide2 only, but then I realized that QtCore.QSocketNotifier is not a function, but a class constructor. I don't think that's currently supported by QtCompat namespacing. https://github.com/mottosso/Qt.py/blob/master/Qt.py#L1259 is a example of how we are adding a decorator for QFileDialog in PyQt4.

Perhaps we could make the QtCompat namespace objects implement a __new__(...) factory which would return a instance of the Qt class when instantiated? This would also take a decorator like the individual functions. The created instance would be vanilla Qt and not have the custom modifications on the QtCompat namespace objects.

# PySide2, untested
>>> from Qt import QtCompat, QtCore
>>> notifier = QtCompat.QSocketNotifier(r_sock.fileno(), QtCore.QSocketNotifier.Read)
>>> print type(Qt.QtCore.QSocketNotifier)
<type 'PySide2.QtCore.QSocketNotifier'>
# this is a unmodified qt class not a QtCompat class

[Ahuge]

I enjoy that proposal, that seems slick

[MHendricks]

https://github.com/mottosso/Qt.py/blob/master/Qt.py#L1000

hmm. I wonder if the existing _build_compatibility_members would work. It's pretty generic, and wonder if adding a "new" key to the _compatibility_members dict would be all that is needed.

If you have time could you try that out? We have finally updated our studio to use Qt.py, so I can't work on it at work like I used to.

[Ahuge]

Yeah I'll give it a go.

I have limited ability at work too, if I don't get to it today, I'll hit it up after work.

[Ahuge]

So looks like if we monkey patch the __new__method like this the C++ object gets lost and we cannot use the object.

My testcase:

def _pyside2():
...

    def _qsocketnotifier_new_wrapper(func_replacement):
        def _fake_new(cls, socket, notify_type, parent=None):
            socket_fd = socket
            import socket
            socket_obj = socket.fromfd(socket_fd, socket.AF_UNIX, socket.SOCK_STREAM)
	    inst = func_replacement(getattr(Qt, "_QtCore").QSocketNotifier, socket_obj, notify_type, parent=parent)
            return inst
            
        _fake_new.__doc__ = func_replacement.__doc__
        _fake_new.__name__ = func_replacement.__name__

        return _fake_new

    decorators = {
        "QSocketNotifier": {
            "__new__": _qsocketnotifier_new_wrapper,
        }
    }

    _reassign_misplaced_members("PySide2")
    _build_compatibility_members("PySide2", decorators)

And when I use that

from Qt import QtCore,QtCompat
import socket

w, r = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM)
notif = QtCompat.QSocketNotifier(r.fileno(), QtCore.QSocketNotifier.Read)

notif.setEnabled(True)
# >>> RuntimeError: Internal C++ object (PySide2.QtCore.QSocketNotifier) already deleted.

I think we have to wait on PySide2 to fix itself.

[MHendricks]

That's too bad. Does anyone have experience with managing PySide2 memory?

Also, socket.socketpair, socket.AF_UNIX, and socket.fromfd are only available on unix.

[mottosso]

We can't fix PySide2, because even if we do we can't change what's shipped with the various DCCs.

If we fix it, best we can hope for is compatibility with Maya 2020 and friends, which isn't ideal IMO. We'll need to stick with whatever is available in the lowest version of the VFX platform we support.

• https://github.com/mottosso/Qt.py/blob/master/Dockerfile.vfxplatform2017
• http://www.vfxplatform.com/

The alternative is dropping a minimum version and chasing the latest build, in which case Qt.py won't be compatible with much at all.

[fredrikaverpil]

I think we have to wait on PySide2 to fix itself.

I'll have to cast my vote on this one. We'd be looking at quite the amount of patches if we didn't take this stand.

[fredrikaverpil]

Maya 2018.3 was released yesterday;

>>> import PySide2
>>> PySide2.__version__
# Result: 2.0.0~alpha0 #

This means Maya is still using a really old version of PySide2 😢

[albertosottile]

To be fair, this code runs just fine on PySide 1.2.4 on macOS 10.10+.

import socket
from PySide import QtCore, QtGui

app = QtGui.QApplication([])

w_sock, r_sock = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM)
notifier = QtCore.QSocketNotifier(r_sock, QtCore.QSocketNotifier.Read)
notifier.setEnabled(True)

print type(notifier)
<type 'PySide.QtCore.QSocketNotifier'>

So, I wonder if suggesting to the PySide2 team to change the syntax was a good idea, given that they just had chosen a different constructor from the beginning (in comparison with PyQt4/PyQy5). In addition, that choice was apparently consistent with Qt (see http://doc.qt.io/qt-5/qsocketnotifier.html#QSocketNotifier).

Incidentally, the patch used to change the constructor is currently causing a segmentation fault on Python 2.7 and macOS (last comment in https://bugreports.qt.io/browse/PYSIDE-629).

If Qt.py is based on the PySide2 syntax, maybe it would be better to embrace it and keep the constructor as it is in PySide1 and as it was in PySide2, so passing the socket. After all, creating a QtCompat member for PyQt4 and PyQt5 is trivial if the constructor takes the socket as one of its argument, while the opposite can be quite challenging.