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

PR: Add support for PySide6 #225

Merged
merged 2 commits into from
Oct 12, 2021
Merged

PR: Add support for PySide6 #225

merged 2 commits into from
Oct 12, 2021

Conversation

jschueller
Copy link
Contributor

Several modules are not available yet and will be re-added later in qt 6.x series:
https://doc-snapshots.qt.io/qt6-dev/whatsnew60.html#removed-modules-in-qt-6-0

@ccordoba12
Copy link
Member

Thanks a lot for your help @jschueller! I'll try to merge your work and release a new version of QtPy during the holidays.

@Czaki
Copy link
Contributor

Czaki commented Jan 5, 2021

both https://pypi.org/project/PySide6/ and pypi.org/project/PyQt6/ was released. @jschueller. May you add PyQt6 to this PR?

@jschueller
Copy link
Contributor Author

hello, I dont have pyqt6 available on my box unfortunately

@Czaki
Copy link
Contributor

Czaki commented Jan 5, 2021

hello, I dont have pyqt6 available on my box unfortunately

it is installable using pip (on Linux you need the most modern pip version)

@jschueller
Copy link
Contributor Author

sorry, I have troubles installing currently

@Czaki
Copy link
Contributor

Czaki commented Jan 5, 2021

So maybe best is to wait for merge and then I prepare next PR base on modified master.

@jschueller
Copy link
Contributor Author

yes, good to merge @ccordoba12 ?

@stonebig
Copy link
Contributor

It would be nice to have an official release with this

@stonebig
Copy link
Contributor

stonebig commented Jan 30, 2021

I think there are some issues:

Line 188, you should rather switch to 'pyside6' not 'pyqt':

qtpy/__init__.py 

image

@stonebig
Copy link
Contributor

stonebig commented Jan 30, 2021

also then I have an issue on Qaction

PySide2:
image

PySide6:
image

so in QtWidgets.py, I would add
image

@stonebig
Copy link
Contributor

stonebig commented Jan 30, 2021

then I get a :

AttributeError: type object 'PySide6.QtCore.Qt' has no attribute 'BackgroundColorRole'

looking at the page https://doc.qt.io/qtforpython/porting_from2.html to digg one step further

answer there:
https://doc.qt.io/qt-5/qt.html
image

So , I would suggest in QtCore.py to add 2 lines:
image

@stonebig
Copy link
Contributor

After this 4 line patch..... WinPython Starts with PySide6 ! .. Small step for man
image

@stonebig
Copy link
Contributor

there is also PyQtGraph that has merged a compatibility patch, that may be of interest
pyqtgraph/pyqtgraph#1495

@stonebig
Copy link
Contributor

looking further for Pyzo, I get a

that seems relateed to https://doc.qt.io/qt-5/qcoreapplication-obsolete.html#flush:
image

@stonebig
Copy link
Contributor

stonebig commented Jan 30, 2021

I'm stucked in QDirModel not working for Pyzo (going to the splash)... if anyone can help, as I'm in Qt like an oliphant in a porcelaine shop.
was supposed to be there https://doc.qt.io/qt-5.12/qdirmodel.html where they suggest to go to QFileSystemModel

https://doc-snapshots.qt.io/qtforpython-dev/PySide2/QtWidgets/QCompleter.html

obsolete note was in 2010... https://www.qt.io/blog/2010/01/08/qdirmodel-is-now-obsolete-qfilesystemmodel-is-taking-the-job

... there was a patch in Pyzo trying to do that change pyzo/pyzo@8190af6

@cmaureir
Copy link

cmaureir commented Feb 3, 2021

Hello 👋
just FYI...due to issues in the CI configuration, we were unable to include Qt3D nor ImageFormats in PySide 6.0.0, which were released in Qt 6.0.0 as 'addons'.

Since many people was asking for it, we decided to include them in the next bug-fix release, to avoid people waiting until 6.1.0.
So hopefully when PySide 6.0.1 is out (next couple of days), it will include both 3d (Qt3DCore, Qt3DAnimation, Qt3DExtras, Qt3DInput, Qt3DLogic, and Qt3DRender) and imageformats.

Sorry if this generate more work for the people working on this task.

@CAM-Gerlach
Copy link
Member

Hey @jschueller and @stonebig , thanks so much for your hard work on this! I've been trying to do my small part to help get this repo back in shape in terms of basic maintenance, tests/CIs, PR review and hopefully cutting releases, since its a package that is relied on by a substantial number of users, whose primary maintainer unfortunately left the Spyder org some time ago. In particular, the most critical piece of the puzzle, PR #208 which overhauls the tests and CI infrastructure, should be pretty much ready to merge now, so once we go ahead right that we'll be able to test and merge other PRs again (since the old CIs were evidently disabled at some point, so we have no CI at all until we merge that).

As contributors here, I really wanted to get your input on how to best move forward. It seems the first priority right now is getting the backlog of PRs reviewed, tested and merged as soon as practical, so we can release 1.10 ASAP with the accumulated changes. Then, what I'm thinking might be a good tentative plan beyond that is to clean house and drop Py2 and Qt4, which we're no longer able to properly test due to package availability, add Qt6 support with this PR and release that shortly after as 2.0, but that's just my internal thinking, How does the timing of this PR fit into that picture, and what are your thoughts on that plan overall?

Would there be value in a release that supports all three generations of Qt (i.e. merging this for 1.10, if its ready) or better to focus on supporting two generations at a time? If we did the former, though, we'd need a way to comprehensively test that this PR hasn't broken Qt4 support significantly on any platform, which right now doesn't appear to be easily possible on our CIs. Obviously, since a lot of things depend on QtPy and its ultimately a compatibility shim, we'd want to minimize any breakage between the releases (i.e. it probably wouldn't be a good idea to move/remove anything that's not Qt4-only just because we dropped Qt4 support), but the new major version would indicate dropping a major version of a Qt (and Python), which is a pretty big change.

@jschueller
Copy link
Contributor Author

I agree qt4 and python2 should be dropped
This does not necessarily require a major version bump: its just regular maintenance imho

@Czaki
Copy link
Contributor

Czaki commented Mar 21, 2021

This does not necessarily require a major version bump: its just regular maintenance imho

It should be connected with the version bump. No one expects a drop of support for something without bump major version.

@stonebig
Copy link
Contributor

Qt Company does a version bump when droping deprecated things (like for PySide6), so doing a version bump also in QtPy may look logical to this eco-system

@CAM-Gerlach
Copy link
Member

Yeah, if it was just dropping or bumping a minor dependency that could be done in a minor release, but this is a fundamental breaking platform change, to both the runtime and the supported core frameworks over which QtPy is a shim. If people are currently using QtPy with Qt4 (or to a lesser extent Py2, since pip does handle this to some extent), a version without such support will no longer work and thus be a breaking API change per SemVer and general practice. This serves as a signal to both automated tooling and human developers that this version should be carefully evaluated for compatibility impacts with their application, and inform their own decisions about what to support and whether/how to bump their own versions.

Furthermore, if we ship this PR with 2.0, it would signal the support of a major new version of the framework it shims, just like PyQt and PySide bump their major versions. We could establish a consistent policy of support for the n and n - 1 major versions of Qt with each QtPy major version, so that both us and downstream users know what to expect and can plan accordingly.

However, we should add a DeprecationWarning for 1.10 if the active Qt API is a Qt4 one, per semver, so users and developers have a warning of what to expect and we can make sure we're not getting a flood of reports about the breakage that would cause.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Mar 23, 2021

Hey @stonebig and @jschueller , @ccordoba12 agrees with our plans (merge outstanding PRs and maintenance and release QtPy 1.10 with them, then drop Py2.7 and Qt4 and merge this for 2.0, once we have CI coverage for Qt6 on at least a decent swath of the matrix) so we can move forward with it. PR #208 is merged to get the tests and CIs overhauled and working again, so you'll want to rebase and fix the merge conflicts (sorry about that! had to fix some tests on our CIs) to pick up those changes.

At some point, preferably before merging this, we'll need to add PyQt 6 and PySide6 to the CIs—feel free to do that here, since you probably are familiar with the details of that due to your WinPython packaging experience, or I can try to help with that too. At least on Windows, AFAIK Qt is bundled in the pip wheels of PyQt and PySide but I don't know if they'll need distro packages on Linux or what's the situation on macOS. Conda avoids these issues, but it seems we're not likely to see Qt6 packages out for at least the near future, with Qt 5.15 still not being out (though Ray has apparently had builds on his channel since August, per conda-forge/pyqt-feedstock#84 ), and we have funding to hopefully get it out by May), and we want to test on both platforms of course.

Thoughts?

@dalthviz
Copy link
Member

Almost forgot, another missing thing is the addition of the scripts/config to setup and test PySide6 in our CI/Github Actions workflow but I think we can help @jschueller with that by either adding the relevant changes here (after the suggestions get accepted) or in another PR.

What do you think @ccordoba12 ?

@ccordoba12
Copy link
Member

Yeah, I think we can merge this one once @jschueller addresses your last comments, then add CI for Pyside6 in a follow-up PR.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Oct 11, 2021

Yeah, I think we can merge this one once @jschueller addresses your last comments, then add CI for Pyside6 in a follow-up PR.

As @ccordoba12 mentions, the latter seems preferable; I can help with that, you want @dalthviz , since I'm also familiar with the CIs having set them up initially. It might be prudent to merge PR #251 first before branching for that one to avoid conflicts and trim down the matrix, and then this followup PR could add Pyside6 support (and potentially PyQt6) as well as bumping the upper bound Python version to 3.9 (or 3.10, once its available on `defaults).

Also, @dalthviz , do we want to milestone this v2.0.0 as previously discussed (like #251 also is)? I linked #229 (PySide6 support) but not sure if this should also close #223 . It would be preferable to add CI support for both at once if this does in fact add it.

@CAM-Gerlach CAM-Gerlach linked an issue Oct 11, 2021 that may be closed by this pull request
@ccordoba12 ccordoba12 added this to the v2.0.0 milestone Oct 11, 2021
Copy link
Contributor

@benoit-pierre benoit-pierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few issues I noticed while trying PyQt6 support.

qtpy/QtDataVisualization.py Outdated Show resolved Hide resolved
qtpy/QtDataVisualization.py Outdated Show resolved Hide resolved
qtpy/__init__.py Outdated Show resolved Hide resolved
@jschueller
Copy link
Contributor Author

jschueller commented Oct 12, 2021

@ccordoba12 I enabled CI, please merge it

Copy link
Contributor

@benoit-pierre benoit-pierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more PyQ6 issues.

qtpy/QtCore.py Show resolved Hide resolved
qtpy/QtWidgets.py Show resolved Hide resolved
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jschueller for all the work and patience! This is awesome 👍

Also, thank you to @altendky @benoit-pierre @CAM-Gerlach @cmaureir @Czaki @kumattau @stonebig (and any other that maybe I'm missing - a lot of comments in this PR) for your help either reviewing/checking or commenting new info to improve and finish this!

@dalthviz
Copy link
Member

dalthviz commented Oct 12, 2021

Also, @dalthviz , do we want to milestone this v2.0.0 as previously discussed (like #251 also is)? I linked #229 (PySide6 support) but not sure if this should also close #223 . It would be preferable to add CI support for both at once if this does in fact add it.

Yep, for v2.0.0 and thank you for linking it to the issue @CAM-Gerlach 👍 About #223 I'm not totally sure if it is related to this. Maybe you mean #233 ? If that is the case still there is some work to do for PyQt6 (here some base work was done but we need to continue and finish that in another PR which I think should add the changes for the corresponding CI support too).

@dalthviz dalthviz merged commit 2f2a25a into spyder-ide:master Oct 12, 2021
@jschueller jschueller deleted the qt6 branch October 12, 2021 14:55
@CAM-Gerlach CAM-Gerlach mentioned this pull request Oct 12, 2021
@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Oct 12, 2021

Yeah thanks for clarifying @dalthviz , I meant #233 — my mistake. That makes sense; whatever you think is best. Let me know if my help is desired. I can open a separate issue for formally declaring and testing 3.9/3.10 support, now that 3.10 is released on AD/defaults and CF as well as upstream and on GH actions.

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.

Pyside6 support