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

Py3fy backend_qt5 + other cleanups to the backend. #11080

Merged
merged 1 commit into from Apr 19, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Apr 18, 2018

Remove unused ToolbarQt._last. Deprecate error_msg_qt and
exception_handler, which were added in 3e315fb (2005) and actually never
used.

PR Summary

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

Remove unused ToolbarQt._last.  Deprecate error_msg_qt and
exception_handler, which were added in 3e315fb (2005) and actually never
used.
@anntzer anntzer added the Py3k label Apr 18, 2018
@anntzer anntzer added this to the v3.0 milestone Apr 18, 2018
@@ -678,7 +675,7 @@ def destroy(self, *args):
self.window.close()

def get_window_title(self):
return six.text_type(self.window.windowTitle())
return self.window.windowTitle()
Copy link
Member

Choose a reason for hiding this comment

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

PyQt type <-> python type:

Should this still be wrapped in a str()? Otherwise we'll return a QString(), which I don't think we want to leak out of the backend API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this actually returns a str (automatic conversion, see http://pyqt.sourceforge.net/Docs/PyQt5/gotchas.html#python-strings-qt-strings-and-unicode), try yourself plt.gcf().canvas.parent().windowTitle()
same comment applies throughout

Copy link
Member

@timhoffm timhoffm Apr 19, 2018

Choose a reason for hiding this comment

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

It works 😄. Not sure I understand why.

The docs on default conversions do only speak of "If Qt expects ...". For me that sounded like parameter conversion. Somehow the return type is converted as well?!? So I never would have the chance of getting the actual QString here? I should really learn about the changes in PyQt5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See also http://pyqt.sourceforge.net/Docs/PyQt4/incompatible_apis.html
We basically don't support the qt4v1 api (too many places where we already assume that we get native strings out of qt methods).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. I learned something new. 😄

@@ -779,7 +776,7 @@ def edit_parameters(self):
item, ok = QtWidgets.QInputDialog.getItem(
self.parent, 'Customize', 'Select axes:', titles, 0, False)
if ok:
axes = allaxes[titles.index(six.text_type(item))]
axes = allaxes[titles.index(item)]
Copy link
Member

Choose a reason for hiding this comment

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

PyQt type <-> python type?

try:
self.canvas.figure.savefig(six.text_type(fname))
self.canvas.figure.savefig(fname)
Copy link
Member

Choose a reason for hiding this comment

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

PyQt type <-> python type:
can savefig handle a QString?

try:
self.canvas.figure.savefig(six.text_type(fname))
self.canvas.figure.savefig(fname)
Copy link
Member

Choose a reason for hiding this comment

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

PyQt type <-> python type?
can savefig handle a QString?

@@ -931,7 +928,6 @@ def __init__(self, toolmanager, parent):
QtWidgets.QToolBar.__init__(self, parent)
self._toolitems = {}
self._groups = {}
self._last = None
Copy link
Member

Choose a reason for hiding this comment

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

Is this just never used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed.

@jklymak jklymak merged commit 0722828 into matplotlib:master Apr 19, 2018
@anntzer anntzer deleted the qtcleanup branch April 19, 2018 18:14
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

3 participants