Skip to content
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

Transition to PySide6 #45

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Transition to PySide6 #45

wants to merge 11 commits into from

Conversation

qku
Copy link
Contributor

@qku qku commented Jan 29, 2023

Description

I played around with qudi-core and the dummies in qudi-iqo-modules trying to make them compatible with PySide6/Qt6.

Most changes are simple replacements of from PySide2 import ... with from PySide6 import .... Points I am unsure of are

  • the QT_API environment variable
  • the changes I made to config.py (d782618) to get rid of a meta class error message

Motivation and Context

I am sure that there are many good reasons to eventually transition to PySide6. Personally, I was motivated to play around with this since I am mostly developing on a laptop with Apple Silicon processor which does not support PySide2.

How Has This Been Tested?

At first, I tried to simply get the manager gui to start. Then, I tried loading all dummy modules from the qudi-iqo-modules default config file.

Types of changes

  • Bug fix
  • New feature
  • Breaking change (Causes existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • I have documented my changes in /docs/changelog.md.
  • My change requires additional/updated documentation.
  • I have updated the documentation accordingly.
  • I have added/updated the config example for any module docstrings as necessary.
  • I have checked that the change does not contain obvious errors
    (syntax, indentation, mutable default values, etc.).
  • I have tested my changes using 'Load all modules' on the default dummy configuration.
  • All changed Jupyter notebooks have been stripped of their output cells.

@qku
Copy link
Contributor Author

qku commented Jan 29, 2023

See also the corresponding PR to qudi-iqo-modules: Ulm-IQO/qudi-iqo-modules#69

@Neverhorst
Copy link
Member

Hi @qku,

thank you for taking an interest in the very foundation of qudi and sorry for the late reply.
I briefly tested the same thing a year ago (see: branch pyside6-dev).
As you have already mentioned in this PR and Ulm-IQO/qudi-iqo-modules#69 you already found a few tripping hazards (signals, environent variables, ...) when transitioning from PySide2 to PySide6.
The problem is, there are many more of these breaking issues that are not always obvious.
Since PySide6 is a mayor overhaul we must expect it to be a breaking change and a transition can only be done after very extensive testing of all qudi components. Simply switching to PySide6 and "hoping for the best" is certainly not a preferable solution.
Since PySide6 is currently not offering substantial improvements for qudi and Qt 5.15. is still in LTS, the transition of qudi to PySide6 is considered rather low-priority.

So what do we need to make the switch?
Extensive unit tests for (at least) the most important qudi-core features!

The transition to PySide6 just highlights yet again how important a proper testing setup is to maintain qudi... and so far it is lacking in that department.
Testing everything "by-hand" is extremely time consuming, error-prone and a recurring task (think about PySide7 and other core changes).
I personally endorse any attempt in transitioning to PySide6 rather sooner than later, but I simply do not have the time to do this on my own.

TL;DR:
The first step in transitioning to PySide6 has been taken now two times in this PR and pyside6-dev.
However the bulk of the work is yet to be done, namely the implementation of extensive unit tests and manual testing of all measurement modules.
For repositories that are not maintained by this project (other than qudi-core and qudi-iqo-modules) a comprehensive documentation needs to be written to serve as guide for developers using qudi to make the transition to PySide6.

@krokosik
Copy link

krokosik commented Sep 5, 2023

@Neverhorst I would argue that PySide6 does bring one considerable improvement. Namely, PySide2 does not support Python 3.11 and it does not look like it will in the near future. All the other packages, like PyQt5 do support it and from what I tested when installing qudi-core from source, PySide2 is the main obstacle for 3.11 support.

Currently, we have our own code for running our lab equipment, but I was looking for some more architecture as it scales up and requires more maintainability. We've recently updated to 3.11 and noticed considerable performance gains when running live graphs using pyqtgraph. Unfortunately, we cannot adopt qudi-core now, as it does not support 3.11.

I understand the importance of tests and a smooth transition process, but since the API is mainly the same between versions, the breaking changes are minimal. On the other hand, maybe you should consider switching to PyQt5? It has the same underlying C++ Qt version, but supports 3.11 with minor tweaks to the API required. I would go even further and utilize a library like qtpy that allows to easily switch between all Qt bindings and supports gradual migrations.

@Neverhorst
Copy link
Member

Neverhorst commented Sep 5, 2023

@wkrasnicki I did not say we do not want to upgrade to PySide6. This step is certainly necessary for the future. It's just that we currently do not have the capacity to make this the number 1 priority. Also, qudi (and PySide2) now supports Python 3.10.x. If a small group of people would be willing to take on this project, I'm more than happy to support this endeavor.

What I would like to avoid (because it always happens over and over again) is implementing a big change and "testing" only being done for "the stuff that I personally need" a.k.a. "works for me". ALL library toolchains need to be tested at least once.

Regarding PyQt5:
The early versions of qudi (before qudi-core) was based on PyQt5. And I agree that t was working slightly better and was better maintained than PySide2. However, we went through great lengths to change the license of qudi from GPL to LGPL with the release of qudi-core.
This was a necessary step to facilitate the spread of qudi from purely academic use to companies (for rapid prototyping, demonstrations, products etc.) while maintaining the open-ness of the project.
Unfortunately PyQt5 was the only dependency with an incompatible license, namely GPL, while PySide2 was licensed under LGPL.
While qtpy sounds good at a first glance, we experienced it to be more of a hassle to maintain the package dependencies in the past.

@krokosik
Copy link

krokosik commented Sep 5, 2023

@Neverhorst I understand. What were your issues with qtpy? I believe this could be a good solution in terms of the current problem, i.e. you providing a GUI framework and users having different needs. For example, right now every modules developer is required to use PySide2 when developing their own modules. If we wanted to migrate our code to qudi we would first have to migrate from PyQt5 (Python version aside). Being Qt binding agnostic would help with that, as it would be the end user's choice to pick the library.

This is already supported in other libraries like matplotlib or pyqtgraph and would allow for example @qku to use PySide6 in their application. Other users, like us, could use PyQt5 without you having to worry about the license. I could help in migrating to qtpy and want to contribute to qudi if we choose to adopt it, which we currently can't because of stated reasons.

@Neverhorst
Copy link
Member

@wkrasnicki Well, back in the day we had lots of trouble with the installation and the configuration of qtpy along with the underlying linkage to the Qt binding (especially if multiple bindings were installed). I was swamped with users not being able to follow installation steps that worked for others, especially when using conda repositories instead of PyPI. Also special care had to be taken when upgrading even minor version numbers of the Qt bindings, because sometimes qtpy needed time to fully support new releases.
Regarding the abstraction we also had different Qt bindings behaving slightly different even though we used the same qtpy feature. So the abstraction was not 100% requiring some manual workarounds here and there that needed maintenance for each package update.
It just multiplies the effort to guarantee/support/maintain a proper installation and package dependency/versioning. In the end we can now more confidently claim to provide a working software package compared to being dependent on the qtpy project and ALL supported Qt bindings. At the expense of reduced flexibility for the user. I mean in the end people come knocking on my door when qudi is not working because they used a Qt binding that is not well tested to work with qudi.

@krokosik
Copy link

krokosik commented Sep 5, 2023

Ah, understandable.

@Neverhorst
Copy link
Member

And I mean for 99% of the use cases transitioning from PyQt5 to PySide2 is mostly changing some imports. We already did it for qudi and that was not a big problem. The bigger problems were in the core modules where some darker Qt magic was happening compared to the measurement modules (GUI, logic, hardware). I would say moving to PySide6 is less work on the long run than supporting all Qt bindings via qtpy.
Using qtpy would also mean that the stability of qudi would - to a large extent - depend on the qtpy and Qt binding developers and not us.

If larger parts of the community are convinced that having a configurable Qt binding in qudi warrants reduced maintenance, support and robustness, I'm certainly open to change my standpoint. Maybe it would be good to open a discussion here so more people can voice their opinion on that and we hopefully get a better picture of the community needs.

Internally we also had longer discussions about that topic years ago so it's not like the team or myself are adamant in this opinion. 😉

@krokosik
Copy link

krokosik commented Sep 5, 2023

I see. In my experience I have mostly dealt with the 99% of cases, you've mentioned ;)

I didn't realize qudi fell in the remaining 1%. I guess we'll switch to PySide when porting our GUIs to qudi then.

@qku
Copy link
Contributor Author

qku commented Sep 6, 2023

In the meantime I found a way to run qudi on macOS with Apple Silicon ARM chip (M1/M2):

  1. create a conda environment configured to use x86 code according to this post on stackoverflow
  2. use a workaround to fix an issue where the main qudi window does not show up - add the line of code to runnable.py, at the start of main()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants