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 PySide2 support #84

Merged
merged 21 commits into from May 8, 2017
Merged

Conversation

dgovil
Copy link
Contributor

@dgovil dgovil commented Oct 26, 2016

Fixes #15


This lets qtpy work with PySide2.
The tests completed on my end and I've manually tested using PySide2, both directly in Python and using Maya 2017 that ships with PySide2.

A few libraries aren't included in some builds of PySide2 by default, but like the other non-default Qt modules, I've left them in as is so that if someone builds PySide2 with them they'll work. Specifically QtMultimedia, QTest and QWebSettings were not available.

I figure it's better to have most of the functionality working than have all of it.

@goanpeca
Copy link
Member

@dgovil this is great thanks a lot :-)

Copy link
Contributor

@Nodd Nodd 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 style comments, mostly...

qtpy/__init__.py Outdated

os.environ.setdefault(QT_API, 'pyqt5')
API = os.environ[QT_API].lower()
assert API in (PYQT5_API + PYQT4_API + PYSIDE_API)
assert API in (PYQT5_API + PYQT4_API + PYSIDE_API+PYSIDE2_API)
Copy link
Contributor

Choose a reason for hiding this comment

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

Spaces around the + operator please !

from PySide2.QtWebKitWidgets import QWebView as QWebEngineView
#: Current PySide2 builds seem to be missing this.
#: I'll leave it in for now because the final builds should have it
from PySide2.QtWebKit import QWebSettings as QWebEngineSettings
Copy link
Contributor

Choose a reason for hiding this comment

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

It means that right now importing QtWebEngineWidgets will fail with pyside2 even if QWebEngineSettings is not used ? I'm in favor of commenting this line for now and readding it when pyside2 includes QWebSettings.

@dgovil
Copy link
Contributor Author

dgovil commented Nov 4, 2016

@Nodd Addressed the notes. Added a newline to the gitignore but for some reason it doesn't seem to survive a push to github. Git status locally shows it's up to date so not sure what's up.

Had to add a comment as the last line for it to show up on github, so hopefully that's alright with you.

@ccordoba12 ccordoba12 added this to the v1.3 milestone Jan 2, 2017
@goanpeca
Copy link
Member

@dgovil do you have any updates on pyside2 status?

@dgovil
Copy link
Contributor Author

dgovil commented Mar 30, 2017

@goanpeca Hi sorry, no I am not aware of what the PySide2 status is as a project. Kind of hard to track when they shifted it off of github.

It ships with commercial products right now in its current state however

@xmikos
Copy link

xmikos commented Mar 30, 2017

One can use Qt.py instead, which supports PySide2 (in fact PySide2 is main binding for Qt.py).

@dgovil
Copy link
Contributor Author

dgovil commented Mar 30, 2017

@xmikos Yup, I actively use Qt.py instead of qtpy primarily for that reason, also because it's easier to package up since its a single file.

Just put this pull request in to try and get parity between the projects.

@tacaswell
Copy link

This is a blocking issue for matplotlib moving to use qtpy over our own shims.

@ccordoba12
Copy link
Member

@tacaswell, really great to know you're considering qtpy for Matplotlib!!

We just need to add tests for this, that's why we haven't merged it yet. I'll work on that during Easter days :-)

@ccordoba12 ccordoba12 changed the title PySide2 support PR: Add PySide2 support May 5, 2017
@ccordoba12 ccordoba12 assigned dalthviz and ccordoba12 and unassigned dalthviz May 5, 2017
@ccordoba12 ccordoba12 force-pushed the PySide2Support branch 7 times, most recently from 6081b5a to b6f584e Compare May 8, 2017 03:01
@ccordoba12
Copy link
Member

Ok, after a lot of work to run our tests with PySide2, this is finally ready!!

Thanks @dgovil for your contribution!!

@ccordoba12 ccordoba12 merged commit 94298fe into spyder-ide:master May 8, 2017
@dgovil
Copy link
Contributor Author

dgovil commented May 8, 2017

Awesome! Thanks for setting up the tests and test environment. Glad I could contribute.

Cheers

@chadrik
Copy link

chadrik commented May 8, 2017

Don't forget to update the README to include PySide2!

@ccordoba12
Copy link
Member

You're right. We'll do it before releasing 1.3, thanks!

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.

None yet

8 participants