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

Fix shims with PyQt5 5.11 #11500

Merged
merged 2 commits into from Jul 3, 2018
Merged

Fix shims with PyQt5 5.11 #11500

merged 2 commits into from Jul 3, 2018

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jun 24, 2018

PR Summary

Fix import of PyQt5 5.11.

sip is not really required by this code when using PyQt5, and 5.11 now no longer requires it nor provides it globally [1].

Also, move the PyQt/PySide shim bits to the end to be near the Qt5/Qt4 shim.

[1] http://pyqt.sourceforge.net/Docs/PyQt5/incompatibilities.html#pyqt-v5-11

Remove unnecessary Qt signal connection.

This fails with PyQt5.11, but is also unnecessary. _destroying is set to True before this signal is connected, so the callback simply returns immediately. Thus, just remove the connection entirely.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

sip is not really required by this code when using PyQt5, and 5.11 now
no longer requires it nor provides it globally [1].

Also, move the PyQt/PySide shim bits to the end to be near the Qt5/Qt4
shim.

[1] http://pyqt.sourceforge.net/Docs/PyQt5/incompatibilities.html#pyqt-v5-11
This fails with PyQt5.11, but is also unnecessary. `_destroying` is set
to True before this signal is connected, so the callback simply returns
immediately. Thus, just remove the connection entirely.
Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

LGTM (the latest pyqt5 release was actually buggy and has been withdrawn, see https://www.riverbankcomputing.com/pipermail/pyqt/2018-June/040459.html, so the build is now unbroken and this can wait for a second reviewer; but in general the PR still looks good).

@anntzer
Copy link
Contributor

anntzer commented Jul 3, 2018

Restarted the build to check with the new release of PyQt5.11.

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This passes the tests, so fixes the QT error all the Travis tests are having...

@timhoffm timhoffm merged commit 2fc375b into matplotlib:master Jul 3, 2018
@jklymak
Copy link
Member

jklymak commented Jul 3, 2018

Let the rebasing begin! Thanks @QuLogic and @anntzer

@tacaswell tacaswell added this to the v2.2.3 milestone Jul 3, 2018
@tacaswell
Copy link
Member

@meeseeksdev backport to v2.2.x

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

5 participants