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: More fixes for Qt 6 compatibility and a PySide2 fix #21685

Merged
merged 7 commits into from
Jan 24, 2024

Conversation

rear1019
Copy link
Contributor

@rear1019 rear1019 commented Jan 9, 2024

Description of Changes

Fixes for Qt 6 compatibility, a PySide2 fix and removal of a Qt 4 check.

Note:

  • PyQt6: Spyder does not start. It can be fixed either in pyqt or in spyder. I will provide more information later.
  • PySide6: Doesn’t work (yet). PySide6 >= 6.5 finally supports super() [1], however, it is flawed (in the sense that it works differently than in PyQt, see [2]). But: I do have some workarounds which allow the GUI to show up, that is, it looks promising after all.

[1] Partially only, in __init__(), which suffices as far as Spyder is concerned.
[2] https://bugreports.qt.io/browse/PYSIDE-2573

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:

rear10

The Qt.NoDropShadowWindowHint flag has been added in Qt 5. Remove an
(implicit) check for Qt 4 now that qtpy dropped support for Qt 4 (in
version 2.0.0). This also ensures consistent behavior in PyQt and
PySide.
@pep8speaks
Copy link

pep8speaks commented Jan 9, 2024

Hello @rear1019! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-01-24 13:51:37 UTC

@rear1019
Copy link
Contributor Author

PyQt6: Spyder does not start. It can be fixed either in pyqt or in spyder. I will provide more information later.

The same issue also affects PySide6. See spyder-ide/qtpy#473 for details.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @rear1019 for your help with this!

All my suggestions are related to the style issues found by pep8 speaks. The rest looks good to me.

@ccordoba12
Copy link
Member

The same issue also affects PySide6. See spyder-ide/qtpy#473 for details.

I took a look at our git history and it's not clear what's the purpose of FakeApplication. However, we wouldn't need to do any changes in QtPy if we remove its exec_ method, right?

If that's the case, then please only remove that method in this PR to see if our tests pass.

Co-authored-by: Carlos Cordoba <[email protected]>
@ccordoba12
Copy link
Member

@rear1019, I think the only missing thing here is to remove the FakeApplication.exec_ method.

@ccordoba12 ccordoba12 changed the title PR: Fixes for Qt 6 compatibility and a PySide2 fix PR: More fixes for Qt 6 compatibility and a PySide2 fix Jan 21, 2024
@rear1019
Copy link
Contributor Author

@rear1019, I think the only missing thing here is to remove the FakeApplication.exec_ method.

@ccordoba12 Sorry for the late reply. I wanted to check something, and something personal came up. Anyway:

The same issue also affects PySide6. See spyder-ide/qtpy#473 for details.

I took a look at our git history and it's not clear what's the purpose of FakeApplication.

Yes, that’s what I figured as well.

However, we wouldn't need to do any changes in QtPy if we remove its exec_ method, right?

Doing that alone won’t help: During startup, create_application() [1] creates [2] a SpyderApplication (which derives from QApplication), performs the monkey patching [3] and returns the SpyderApplication instance. Note that SpyderApplication is a subclass of the original QApplication, not the monkey patched FakeApplication. Later on the original exec_() is called, which is an alias to possibly_static_exec() [4]. That helper function makes use of isinstance(), which is now confused by the monkey patch.

I have pushed a commit which completely removes FakeApplication and the monkey patch.

[1]

def create_application():

[2]
app = qapplication()

[3]
QtWidgets.QApplication = FakeQApplication

[4] https://github.com/spyder-ide/qtpy/blob/f8f3422a2c8dac3cc235f79d0d1db28d5dd1cf1c/qtpy/_utils.py#L45

@ccordoba12
Copy link
Member

Thanks for your thorough investigation @rear1019! Since FakeApplication doesn't seem really necessary, I agree with removing it.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thank @rear1019 for all your efforts to get us ready for Qt6!

@ccordoba12 ccordoba12 merged commit 40fbe52 into spyder-ide:master Jan 24, 2024
14 checks passed
hmaarrfk added a commit to hmaarrfk/spyder that referenced this pull request Jun 22, 2024
I followed the work done in spyder-ide#21685

PySide6 still doesn't work. tested with 6.7.1
hmaarrfk added a commit to hmaarrfk/spyder that referenced this pull request Jun 25, 2024
I followed the work done in spyder-ide#21685

PySide6 still doesn't work. tested with 6.7.1
@rear1019 rear1019 deleted the qt6-pyside2-fixes branch February 17, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants