-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
QtCore.QSocketNotifier socket param is different in PySide2 #278
Comments
What do we think we should do here?
|
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) |
I was going to say number 2 would be easy to implement by adding a QtCompat namespace decorator using the code in Perhaps we could make the QtCompat namespace objects implement a # 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 |
I enjoy that proposal, that seems slick |
https://github.com/mottosso/Qt.py/blob/master/Qt.py#L1000 hmm. I wonder if the existing 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. |
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. |
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. |
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. |
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.
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. |
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. |
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 😢 |
To be fair, this code runs just fine on PySide 1.2.4 on macOS 10.10+.
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. |
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:
The text was updated successfully, but these errors were encountered: