ScopeFoundry / ScopeFoundry

ScopeFoundry: A Python platform for controlling custom laboratory experiments and visualizing scientific data
http://www.scopefoundry.org
BSD 3-Clause "New" or "Revised" License
35 stars 19 forks source link

connect() name mangling when using PySide #46

Open rbrenesh opened 6 months ago

rbrenesh commented 6 months ago

When using PySide, apps that successfully run with PyQt will fail, with errors similar to:

self.connection_failed.connect(self.on_connection_failed)
TypeError: connect() takes 1 positional argument but 4 were given

After some searching around, I found that this is because PySide has a connect() function that gets overwritten with HardwareComponent's connect(). If I change connect() in HardwareComponent to, for example, connectFunc() and rename all my hardware implementations from connect() to connectFunc(), the error goes away and the app runs fine.

However, this would involve renaming every single hardware connect() implementation to a new name, which is tedious and not ideal. Not sure if there is a better way to deal with this.

Tested with PySide6 and PySide2

edbarnard commented 6 months ago

Thanks Roberto for finding this! This is a potentially annoying issue that I don't have a quick answer for. We could deprecate the connect() and disconnect() functions and rename them to connect_hw(), disconnect_hw(), but I'm trying think of a way that we don't have to break everyone's code. One possibility is not having the HardwareComponent inherit from QObject, but rather have the QObject as an instance variable inside the HardwareComponent

edbarnard commented 6 months ago

Another potential option would be to use the qtpy wrapper library for any calls to QT functions. I have used it with ScopeFoundry to abstract away the differences. It looks like we currently do for the super class of HardwareComponent but maybe its a leaky abstraction. We could raise the issue for there, and maybe a qtpy solution would solve this issue

https://github.com/spyder-ide/qtpy

rbrenesh commented 6 months ago

I think it goes a bit beyond qtpy. Searching a bit more, I found other people run into similar issues using PySide directly.

PySide documentation specifies that QtCore.QObject has a connect() function. It's annoying that inheriting from QObject seems to modify the underlying definition of connect() for further usage of that class.

I'll still raise the issue with qtpy, but I'm pretty sure they might just defer to this being an issue with PySide. I can raise it with PySide too, but that might not be a fix for quite a while assuming they even get to it.

rbrenesh commented 6 months ago

Thanks Roberto for finding this! This is a potentially annoying issue that I don't have a quick answer for. We could deprecate the connect() and disconnect() functions and rename them to connect_hw(), disconnect_hw(), but I'm trying think of a way that we don't have to break everyone's code. One possibility is not having the HardwareComponent inherit from QObject, but rather have the QObject as an instance variable inside the HardwareComponent

I think this is the easiest way. I noticed LoggingQTextEditHandler also has a problem with its emit function. This StackOverflow has a similar approach.

Let me fork the repo and try to work on a fix. I'll PR once I have something that I've tested as best I can.