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: Replace custom implementation with loadUiType from PySide6 #440

Merged
merged 2 commits into from
Jul 12, 2023
Merged

PR: Replace custom implementation with loadUiType from PySide6 #440

merged 2 commits into from
Jul 12, 2023

Conversation

JaRoSchm
Copy link
Contributor

@JaRoSchm JaRoSchm commented Jul 6, 2023

This replaces the custom implementation of qtpy.uic.loadUiType with the one from PySide6. Fixes #439.

@ccordoba12 ccordoba12 requested a review from dalthviz July 6, 2023 14:21
@ccordoba12 ccordoba12 added this to the v2.4.0 milestone Jul 6, 2023
@dalthviz dalthviz changed the title Replace custom implementation with loadUiType from PySide6 PR: Replace custom implementation with loadUiType from PySide6 Jul 7, 2023
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 @JaRoSchm for working on the fix! Could you also as part of this PR try to remove the skip for the test related with loadUiType at

@pytest.mark.skipif(
PYSIDE2 or PYSIDE6,
reason="PySide2uic not consistently installed across platforms/versions")

Other than that this LGTM 👍 Thank you again!

@JaRoSchm
Copy link
Contributor Author

JaRoSchm commented Jul 8, 2023

Okay, looks like the pyside6-uic command which is necessary for PySide6.QtUiTools.loadUiType (see https://doc.qt.io/qtforpython-6/PySide6/QtUiTools/loadUiType.html) is not contained in the conda-forge version of PySide6 6.4.3 for Linux and macOS. Locally I use PySide6 6.5.1 on Ubuntu also installed from conda-forge and here it is present. Later I will try to find out more systematically in which versions it is present. This is probably what is ment by "PySide2uic not consistently installed across platforms/versions". This could mean that we should still skip the test (and sadly no vitables for my setup right now without larger changes on their side).

@JaRoSchm
Copy link
Contributor Author

JaRoSchm commented Jul 8, 2023

For context there was some discussion about this in #339, #265, #262.

@dalthviz
Copy link
Member

dalthviz commented Jul 10, 2023

Thank you for checking the skip @JaRoSchm ! Could you update the test skip then to follow up your findings and update the reason string to take into account the need for pyside6-uic command?

If that ends being up too complicated or you end up needing help let me know and I will try to help checking more in detail the skip definition 👍

@JaRoSchm
Copy link
Contributor Author

I asked in conda-forge/pyside2-feedstock#193 where pyside6-uic should be available and it turned out that it is only packaged for Linux and macOS in version >= 6.5. QtPy uses 6.4 for its tests.

I tried to include this into the skip condition but somehow this only fixed the Python 3.7 tests. I don't get why the condition isn't true under Python 3.11. Could you please help me there?

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 again @JaRoSchm for all the effort here! Left a suggestion for the tests which hopefully will make the checks green🤞🏼

qtpy/tests/test_uic.py Outdated Show resolved Hide resolved
@JaRoSchm JaRoSchm requested a review from dalthviz July 12, 2023 07:28
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 @JaRoSchm !

@dalthviz dalthviz merged commit 9d1f3ed into spyder-ide:master Jul 12, 2023
@JaRoSchm JaRoSchm deleted the fix_loadUiType_PySide6 branch July 13, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

qtpy.uic.loadUiType failing for PySide6
3 participants