OpenCue icon indicating copy to clipboard operation
OpenCue copied to clipboard

[cuegui] Upgrade to PySide6

Open bcipriano opened this issue 3 years ago • 2 comments

WIP

Open questions

  • Is our saved app config compatible between PySide 2 and 6? It looked like maybe not -- I had to delete my save file from disk and let PySide6 start from scratch, but it's hard to know if that was related to whatever changes I had in-flight at the moment. Needs more research, and we might need to do some special handling of that case.

Link the Issue(s) this Pull Request is related to. Fixes #1194.

Summarize your change. Upgrade all GUI code to use PySide6.

Some objects have changed locations and deprecated features we're using have been removed.

bcipriano avatar Sep 14 '22 19:09 bcipriano

Some initial notes on this change:

  • This PR is currently a mess because I had to merge several other branches in order to test in my local environment. These will be cleaned up as I get those other PRs merged.
    • #1185,
    • #1193,
    • #1198,
    • #1199,
    • #1201
  • PySide6 version support is a bit all over the place right now.
    • On MacOS: 6.3.2 seems to work well. 6.2.x appears to have some serious issues -- stylesheets are not applied as expected, making the app basically unusable.
    • On existing CI pipelines, i.e. CentOS 7: 6.3.x is not available from PyPI. 6.2.x is not able to import due to some libstdc++ incompatibilities. I tried a few methods to upgrade but was unsuccessful -- those ASWF images are based on centos:7, so options are limited. I don't really think that getting this working on CentOS 7 is a priority, given Cent is EOL now.
    • On a new CI pipeline using AlmaLinux (essentially a CentOS replacement), 6.3.2 is available and appears to work well -- tests pass. That image also includes Python 3.9 by default which is great.
  • The current code requires PySide6. We know that for some amount of time, we will need to support both, due to VFX reference platform continuing to use Qt5 until CY2024. We'll have to address this somehow later, most likely through some sort of compatibility layer. But let's ensure that the PySide6-only code is working well first, and we can take it from there.

bcipriano avatar Sep 19 '22 17:09 bcipriano

@DiegoTavares I think this is ready for testing, though it would be better to get those other PRs tested and merged before digging in too much here. That way we can clean up the PR and ensure if we find an issue, it's due to PySide6 and not some other new code.

bcipriano avatar Sep 19 '22 17:09 bcipriano

Working on building a pyside6 package internally to test this.

DiegoTavares avatar Sep 28 '22 23:09 DiegoTavares

We're not going to be able to test this internally.

DiegoTavares avatar Dec 07 '22 22:12 DiegoTavares

From the TSC meeting: per Diego's last comment, we don't have a straightforward way to test this in a full production environment. We plan to proceed like this: add the PySide2/6 compatibility layer to this PR, test as best we can, and merge it as an experimental feature. Issues may still be present but we can fix these as we go. This will be strictly a better situation than we currently have, where CueGUI is not available at all for users who can't access PySide2.

Over time, we will work out any issues with PySide6 support, and it will eventually become a non-experimental feature. Due to the VFX reference platform's planned QT support, we will continue to support PySide2 for years at least.

Also, I've removed the changes contained in https://github.com/AcademySoftwareFoundation/OpenCue/pull/1199 from this PR, as we don't plan to merge that PR at this time.

bcipriano avatar Dec 12 '22 19:12 bcipriano

Hey Brian. Following up from a comment on this that I made in the TSC meeting, I discovered https://peps.python.org/pep-0508/#environment-markers this past weekend. Would be useful for this case where you wanted to be using PySide2 or PySide6 as a dependency depending on the platform/os into which the package is being installed.

ddneilson avatar Jan 24 '23 15:01 ddneilson

We decided to abandon this change in favor of #1238, using the QtPy library.

bcipriano avatar Mar 21 '23 17:03 bcipriano