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: Remove Qt4 support #252

Merged
merged 1 commit into from
Oct 15, 2021
Merged

PR: Remove Qt4 support #252

merged 1 commit into from
Oct 15, 2021

Conversation

jschueller
Copy link
Contributor

@jschueller jschueller commented Oct 12, 2021

Removes Pyside and PyQt4 support (its not even tested anymore), Qt5 is way more common nowadays and we have support for Qt6 (PySide6, and preliminary work for PyQt6)

@jschueller jschueller force-pushed the rip_qt4 branch 2 times, most recently from 99b43f2 to 021edfd Compare October 12, 2021 15:42
@ccordoba12 ccordoba12 requested a review from dalthviz October 12, 2021 15:48
@ccordoba12 ccordoba12 added this to the v2.0.0 milestone Oct 12, 2021
@ccordoba12 ccordoba12 changed the title Nuke Qt4 support PR: Remove Qt4 support Oct 12, 2021
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.

Thanks for all the work @jschueller ! I left some comments in files where there are still some references to PyQt4 and/or PySide (mostly in docstrings or file descriptions).

Besides that, some logic related to PyQt4 still remains at test_main.py and conftest.py.

Also, we need to check the patch for QComboBox (qcombobox.py and test_patch_combobox.py) which maybe should be removed since from the test definition segfaults in PySide2/PySide6 and partially for PyQt5.

Regarding the QComboBox patch what do you think @ccordoba12 ?

qtpy/QtGui.py Show resolved Hide resolved
qtpy/QtWidgets.py Show resolved Hide resolved
qtpy/__init__.py Outdated Show resolved Hide resolved
qtpy/__init__.py Show resolved Hide resolved
qtpy/tests/test_uic.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
@ccordoba12
Copy link
Member

Regarding the QComboBox patch what do you think @ccordoba12 ?

I agree, I think we should remove it.

@jschueller
Copy link
Contributor Author

ok, I removed that patch and its test in the sources

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.

Thanks @jschueller for the changes ! I was checking again and I left a comment in compat.py regarding the _qfiledialog_wrapper

After that I think this should be ready to be merged 👍

Although. just in case, what do you think @ccordoba ?

qtpy/compat.py Show resolved Hide resolved
@ccordoba12
Copy link
Member

Although. just in case, what do you think @ccordoba12?

Looks good to me, so I'd also say it's ready for merge (except for your last comment, of course).

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.

Thanks @jschueller for all the work done here! This LGTM 👍

@dalthviz dalthviz merged commit 3ce89db into spyder-ide:master Oct 15, 2021
@jschueller jschueller deleted the rip_qt4 branch October 15, 2021 15:42
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.

3 participants