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

PyQt6 support #233

Closed
bgermann opened this issue Mar 17, 2021 · 36 comments · Fixed by #294
Closed

PyQt6 support #233

bgermann opened this issue Mar 17, 2021 · 36 comments · Fixed by #294

Comments

@bgermann
Copy link

Does QtPy support the newly released PyQt6 package?

Related: #229

@stonebig
Copy link
Contributor

stonebig commented Apr 15, 2021

I see an interesting metho in PyQtGraph, but I fail to apply it in QtPy context
https://github.com/pyqtgraph/pyqtgraph/blob/master/pyqtgraph/Qt.py#L331..L349

for QtConsole, we have to move all keyboard keys, for example:
Qt.Key_Left = Qt.Key.Key_Left

@stonebig
Copy link
Contributor

PyQtGraph is pretty much leading the change pyqtgraph/pyqtgraph#1707

@The-Compiler
Copy link

The-Compiler commented Apr 16, 2021

FWIW the easiest approach (and what I will do in qutebrowser) is to:

From what I can tell, PySide2 has always allowed scoped access. PyQt5 5.10 and earlier hasn't, and neither has PyQt4 (not sure about the Qt4 PySide). But it's really time to drop those to be honest...

@tlambert03
Copy link
Contributor

Would just like to ping this issue and #229 ...

we (napari org) have been using qtpy and would like to continue doing so but the recent activity in this repo leads one to wonder whether the project is abandoned?

It's my impression that there is some amount of community willingness to help with the transition to Qt6, but a higher level design spec is likely needed to guide some design decisions (see for example #225 (comment) from March, looking for some general input on the desired interface). This requires some input from spyder-ide devs or whoever is maintaining qtpy these days (@dalthviz ... I see you've recently been committing, is this something you can weigh in on?).

Thanks for all the past work!

@ccordoba12
Copy link
Member

Hey @tlambert03, unfortunately we've been very busy in Spyder during the last year and a half, and the people that stepped in in the meantime to help us maintain Qtpy became busy with other things.

@dalthviz is now going through the backlog of PRs we've accumulated. The idea is to merge first the support for PySide6 (hopefully in a couple of weeks) and then add the one for PyQt6.

see for example #225 (comment) from March, looking for some general input on the desired interface

We've always used PyQt as a reference. I'll leave a comment there about it.

@tlambert03
Copy link
Contributor

thanks for the response!

@j9ac9k
Copy link

j9ac9k commented Sep 10, 2021

pyqtgraph maintainer here; trying to maintain qt4 and qt6 will be tough. The enums namespace that PyQt6 requires I don't believe works for Qt4 bindings.

In case it's helpful, here is where we do the bulk of our abstraction layer work (please keep in mind we don't support Qt4 any more): https://github.com/pyqtgraph/pyqtgraph/blob/master/pyqtgraph/Qt/__init__.py

EDIT: just read @The-Compiler 's comment above; and I'm 100% in agreement, especially about not even trying to support PyQt6 < 6.1.

@tlambert03
Copy link
Contributor

Yeah, I'm all for @The-Compiler's proposal too. Including dropping support for pre 5.11

@ccordoba12
Copy link
Member

pyqtgraph maintainer here; trying to maintain qt4 and qt6 will be tough. The enums namespace that PyQt6 requires I don't believe works for Qt4 bindings.

Hey @j9ac9k, thanks for chiming in! Ok, we'll remove support for PyQt4 before adding support for PyQt6/PySide6.

Yeah, I'm all for @The-Compiler's proposal too. Including dropping support for pre 5.11

Ok, we'll take a look at that too.

@j9ac9k
Copy link

j9ac9k commented Sep 10, 2021

Regarding minimum Qt version to support; a frequent pain point we have had is that on the conda defaults channel; Qt 5.9 is what is distributed, so to make current versions of pyqtgraph work w/ conda dependents, conda-forge must be installed.

That said, PySide2 < 5.11 was a real head ache to test for back when we technically supported it. We eventually gave up, and made PySide2 be 5.12+.

@tlambert03
Copy link
Contributor

Qt 5.9 is what is distributed [on the default channel], so to make current versions of pyqtgraph work w/ conda dependents, conda-forge must be installed.

yeah, this issue comes up for us at napari a lot as well, though we also now only support ≥5.12 and try to help users navigate this annoyance in other ways.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Oct 12, 2021

@ccordoba12 Since defaults is still stuck on Qt 5.9.x without a clear public plan I can find for upgrading, we should think carefully about the implications for Spyder. Dropping support for Qt <5.11 means that either Spyder will still have to support QtPy <=1.11.x indefinitely (if and when defaults is upgraded to a later supported version), or defaults will be permanently stuck on an increasingly outdated version of Spyder.

In the latter scenario, either Anaconda will be bundling a long-unsupported version of Spyder (that is still perhaps unfortunately the way a large fraction, if not the majority of Spyder's userbase runs it, a lot of which will take a long time if not forever to change due to institutional policies at many schools, universities, companies and organizations that allow Anaconda but not arbitrary software), which will generate lots of extra issues/support requests, or they would drop it completely, greatly decreasing the Spyder userbase and the ability for users (especially beginners and students) to access it. There's also the small but non-trivial fraction of users where Spyder doesn't work on PyQt 5.12 or even 5.9 on their machines due to various issues, but does under earlier versions (not sure how common that is, but it certainly happened quite a bit in the past), and the fact that past Qt 5.12, LTS updates are no longer open source.

Do you know what the plan is for Qt in AD? If not, maybe we can ask Ray (if he's still there)?

@ccordoba12
Copy link
Member

Dropping support for Qt <5.11 means that either Spyder will still have to support QtPy <=1.11.x indefinitely

Yeah, I'm aware of that. But I don't see too much of an issue with it because we don't use fancy Qt/PyQt features. So we just need to add an upper constraint on QtPy 2.0 in our next version and keep it until Anaconda updates its packages. That way we won't block the community from using the latest PyQt6/PySide6 through QtPy to develop the apps they want.

Do you know what the plan is for Qt in AD?

Quansight is talking to them to see if the situation can be satisfactorily solved during the next months.

If not, maybe we can ask Ray (if he's still there)?

Sadly, Ray passed away several months ago.

@CAM-Gerlach
Copy link
Member

So we just need to add an upper constraint on QtPy 2.0 in our next version and keep it until Anaconda updates its packages.

Okay, I see—but wouldn't it be better to add qt >=5.11 to the run_constrained of the conda recipe for QtPy 2.0.0+ to prevent it from being installed/upgraded in the presence of an unsupported Qt version, and resulting in a broken user environment? This would solve the issue not only for Spyder, but all of the other applications that depend on QtPy, both when being installed/updated in user environments that only have Qt <5.11 installed or available, and for those that don't (yet) support Qt >=5.11 in the first place.

Also, constraining Spyder to not use QtPy 2.0 only affects future Spyder versions, unlike the previous, so broken environments/Spyder installs can still result by several potential avenues (if QtPy 2.0.0 goes live on defaults or conda-forge before the next Spyder version with that change, if Anaconda users install/update to any prior version, or if the solver selects Spyder 5.1.5 and QtPy 2.0.0 instead of Spyder 5.2.0 and QtPy 1.11.2, which are both equally valid possibilities). It would mean CF versions of Spyder cannot benefit from newer Qt/QtPy versions, and Spyder would conflict with any future CF package that required Qt6 or QtPy 2.0.0+. And that's just assuming only the CF version is constrained (which actually doesn't need to be at all); doing the constraint for all versions has even wider-reaching effects on numerous platforms that don't have this problem. Whereas, the former approach would avoid all of these issues. So unless I really misunderstand something, I'd highly recommend the former approach.

Quansight is talking to them to see if the situation can be satisfactorily solved during the next months.

Okay; hopefully things work out with that.

Sadly, Ray passed away several months ago.

Oh wow, I had no idea. That's really sad; he did so much for the community and I always enjoyed interacting with him.

@kumattau
Copy link
Contributor

kumattau commented Oct 19, 2021

Switch all enum access to the scoped variants (I wrote a quick&dirty script to do so)

Is it mean that, for example, Qt.Key_Return cannot be used and Qt.Key.Key_Return must be used instead ?

qtawesome has Qt.Key_Return and some similar "unscoped" enum access.

I think it is better that QtPy supports "unscoped" enum access if possible as the following reasons.

For example, by adding the support of "unscoped" enum access like as following commit, qta-browser with qtpy and QT_API=pyqt6 works.

kumattau@b2d44e4

Is it impractical to add code like the one above in whole PYQT6 of QtPy ?

@j9ac9k
Copy link

j9ac9k commented Oct 19, 2021

PyQtGraph supported the unscoped enums for a while but the performance hit on the initial load was quite substantial (3 seconds or so), so we did away with it.

Since all bindings from 5.12+ (they may be supported in older versions, I just didn't check) supported the scoped enums we had no comparability issues migrating over. A few enums did get missed, but for the most part the automated tooling caught most users. uses.

The tricky thing is that only pyqt6 does not support the unscoped enums, so you must test with pyqt6 too make sure you have the namespace right.

@kumattau
Copy link
Contributor

kumattau commented Oct 19, 2021

How about exec_() ?

exec_() has been removed in PyQt6, but I think it is better that QtPy supports exec_() because the latest PySide2 5.15.2 supports exec_() only.

I think we can simply assign exec to exec_ : QApplication.exec_ = QApplication.exec

@benoit-pierre
Copy link
Contributor

It's not just QApplication, there's also QMenu and QDialog.

@tomgoddard
Copy link

I tried current qtpy github source (Oct 28, 2021) with PyQt 6.2.1 using another PyPi package qtconsole and found one thing missing in qtpy. It needs in Slot defined in QtCore.py

from PyQt6.QtCore import pyqtSlot as Slot

as is currently done for PyQt5.

One other difficulty I encountered is that QMouseEvent has globalPos() returning QPoint in PyQt5, but this is not in PyQt6 where a similar method globalPosition() is available returning QPointF. I don't have a good idea how qtpy should handle the globalPos() method missing from PyQt6.

@j9ac9k
Copy link

j9ac9k commented Oct 29, 2021

One other difficulty I encountered is that QMouseEvent has globalPos() returning QPoint in PyQt5, but this is not in PyQt6 where a similar method globalPosition() is available returning QPointF. I don't have a good idea how qtpy should handle the globalPos() method missing from PyQt6.

The mouse event one is something we (pyqtgraph) frequently get bit with.

There is a fantastic summary on this various "options" pyqtgraph/pyqtgraph#1875 (comment)

A frequent line in our codebase is

position = event.position() if hasattr(event, 'position') else event.localPos()

@tomgoddard
Copy link

Thanks! I also use that kind of code in the molecular visualization application I develop ChimeraX. Of course the point of qtpy as a shim is to hide the differences between different PyQt and PySide versions but in some cases that is not feasible and the client application will need to handle the differences as in your example.

@j9ac9k
Copy link

j9ac9k commented Oct 29, 2021

Hmm...this came up in the mail list not long ago

image

EDIT: @tomgoddard I just took a look at ChimeraX, that's amazing!

@dalthviz dalthviz reopened this Oct 29, 2021
@dalthviz
Copy link
Member

I tried current qtpy github source (Oct 28, 2021) with PyQt 6.2.1 using another PyPi package qtconsole and found one thing missing in qtpy. It needs in Slot defined in QtCore.py

Hi @tomgoddard would you like to submit a PR adding the alias for pyqtSlot on PyQt6? Let us know !

@dalthviz
Copy link
Member

Hmm...this came up in the mail list not long ago

Just checking and yes quite a difference there:

PyQt6 6.2.0 vs PyQt5 5.15.5

imagen

@dalthviz
Copy link
Member

Also, after some discussion with @ccordoba12 we will try to support unscoped enums access in order to keep things working as if you were using PyQt5 (where scoped and unscoped access is available at least from PyQt 5.9.2 + sip 4.9.13 with conda)

@j9ac9k
Copy link

j9ac9k commented Oct 29, 2021

Also, after some discussion with @ccordoba12 we will try to support unscoped enums access in order to keep things working as if you were using PyQt5 (where scoped and unscoped access is available at least from PyQt 5.9.2 + sip 4.9.13 with conda)

In case it helps, here is our logic we used to "promote" enums in PyQt6

https://github.com/pyqtgraph/pyqtgraph/blob/pyqtgraph-0.12.1/pyqtgraph/Qt.py#L331-L377

EDIT: please profile this, when I investigated it took us 3 seconds at import-time. we didn't make any attempt to speed that up at all; so there are likely places for improvement.

@kumattau
Copy link
Contributor

kumattau commented Dec 2, 2021

Does QtPy supports Qt6/PyQt6/PySide6 Version 6.0 ?
I think, if QtPy supports 6.2 or later only, it is better that QtPy shows warning messages like as #284.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Dec 2, 2021

Seems like there was a clear consensus here for not supporting anything earlier than 6.2, so if @dalthviz agrees I can go ahead with a PR to also warn on those versions (I initially added it in a way that would make it relatively straightforward to do so without much code duplication).

@dalthviz
Copy link
Member

dalthviz commented Dec 3, 2021

Sounds good to me, thanks @kumattau for the feedback and @CAM-Gerlach for helping with the implementation 👍

@CAM-Gerlach
Copy link
Member

Thanks, opened as #294

@dalthviz , just to check in, is there anything else specific we need for this issue?

@dalthviz
Copy link
Member

dalthviz commented Dec 6, 2021

@dalthviz , just to check in, is there anything else specific we need for this issue?

I don't think so @CAM-Gerlach . Maybe #294 should close this 👍

@CAM-Gerlach
Copy link
Member

Okay, thanks! Let's hope it resolves this for good this time. 🤞

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

Successfully merging a pull request may close this issue.