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

Implement backend for PyQt5 + modify Qt4 backends to use Qt5 module via shim #3072

Merged
merged 5 commits into from Jun 27, 2014

Conversation

Projects
None yet
9 participants
@mfitzp
Copy link
Member

mfitzp commented May 18, 2014

A backend for PyQt5 based on intial work by @badders, modified to fix Qt5 mouse event handling, then re-structured to implement as a wrapper over the existing Qt4 code. Following discussions on this pull request:

#2471

the code has been restructured to implement PyQt5 backend as a first-class implementation, with other Qt backends (PyQt4, PyQt4v2, PySide (Qt v4)) wrapping it and modifying as required. The issues of objects being moved around in the Qt namespace (many QtGui objects now in QtWidgets) QtWidgets is simply assigned as a copy of QtGui if not available. This achieves the intended outcome with the minimum code.

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

It needs to be re-based an current master.

fn_name, cursord, draw_if_interactive, _create_qApp, show, \
new_figure_manager, new_figure_manager_given_figure, \
TimerQT, FigureCanvasQT, MainWindow, FigureManagerQT, NavigationToolbar2QT, \
SubplotToolQt, error_msg_qt, exception_handler

This comment has been minimized.

@tacaswell

tacaswell May 18, 2014

Member

minor style quibble, this should be done with () continuation not \ (see the from __future__ import).

import matplotlib.backends.qt4_editor.formlayout as formlayout
from matplotlib.backends.qt4_compat import QtGui
import matplotlib.backends.qt_editor.formlayout as formlayout
from matplotlib.backends.qt_compat import QtGui

This comment has been minimized.

@tacaswell

tacaswell May 18, 2014

Member

If I am understanding things correctly, we should not use QtGui internally anywhere so this (and all it's uses) should be changed to QtWidget? Never mind, I was confused

@tacaswell tacaswell added this to the v1.4.0 milestone May 18, 2014

@tacaswell tacaswell referenced this pull request May 18, 2014

Closed

Qt5 Backend #2471

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

There are some issues with the super calls (snipped off the top part of the traceback)

/home/tcaswell/python_env/py2k-clean/local/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/pyplot.pyc in figure(num, figsize, dpi, facecolor, edgecolor, frameon, FigureClass, **kwargs)
    432                                         frameon=frameon,
    433                                         FigureClass=FigureClass,
--> 434                                         **kwargs)
    435 
    436         if figLabel:

/home/tcaswell/python_env/py2k-clean/local/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backends/backend_qt5agg.pyc in new_figure_manager(num, *args, **kwargs)
     45     FigureClass = kwargs.pop('FigureClass', Figure)
     46     thisFig = FigureClass(*args, **kwargs)
---> 47     return new_figure_manager_given_figure(num, thisFig)
     48 
     49 def new_figure_manager_given_figure(num, figure):

/home/tcaswell/python_env/py2k-clean/local/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backends/backend_qt5agg.pyc in new_figure_manager_given_figure(num, figure)
     51     Create a new figure manager instance for the given figure.
     52     """
---> 53     canvas = FigureCanvasQTAgg(figure)
     54     return FigureManagerQT(canvas, num)
     55 

/home/tcaswell/python_env/py2k-clean/local/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backends/backend_qt5agg.pyc in __init__(self, figure)
    175         if DEBUG:
    176             print('FigureCanvasQtAgg: ', figure)
--> 177         FigureCanvasQT.__init__(self, figure)
    178         FigureCanvasAgg.__init__(self, figure)
    179         self.drawRect = False

/home/tcaswell/python_env/py2k-clean/local/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backends/backend_qt5.pyc in __init__(self, figure)
    229 
    230         # NB: Replacing with super to avoid a TypeError: __init__() takes exactly 2 arguments (1 given) on QWidget PyQt5
--> 231         super(FigureCanvasQT, self).__init__(figure=figure)
    232         #QtGui.QWidget.__init__(self)
    233         #FigureCanvasBase.__init__(self, figure)

AttributeError: 'figure()' is not a Qt property or a signal


@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented May 18, 2014

Thanks @tacaswell
I probably got confused in the rearrangement. I will get on a PyQt4 install in a bit and debug, make the style changes and rebase as requested.

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

I was poking at it a bit and I think it is related to the changes with how coperative mix-ins are handled. I suspect that we may end up having to touch more-or-less every class in the canvas tree.

The above was with pysides and am set up to test both pyqt4 and pysides.

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented May 18, 2014

Thanks for the feedback/suggestions.

The latest commit works on PyQt4 (tested on Ubuntu Linux/python2.7). I basically copied the init block across into backend_qt4.py and used the original super-style call. Since the backend_qt5.py was the only place using super() I tried to get rid of that too but for a reason I can't quite fathom it gets upset that 'self' is of the wrong type.

However, the end result is that this should now happily work with both PyQt5 and PyQt4. I'm going to look at installing PySide on the Linux box to test that too.

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

copy-pasta?

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented May 18, 2014

In backend_qt4.py:

def __init__(self, figure):
    if DEBUG:
        print('FigureCanvasQt qt4: ', figure)
    _create_qApp()

    # Note different super-calling style to backend_qt5
    FigureCanvasBase.__init__(self, figure)
    QtGui.QWidget.__init__(self)

    self.figure = figure
    self.setMouseTracking(True)
    self._idle = True
    # hide until we can test and fix
    #self.startTimer(backend_IdleEvent.milliseconds)
    w, h = self.get_width_height()
    self.resize(w, h)

In backend_qt5.py:

    # NB: Replacing with super to avoid a TypeError: __init__() takes exactly 2 arguments (1 given) on QWidget PyQt5
    super(FigureCanvasQT, self).__init__(figure=figure)
    self.figure = figure
    self.setMouseTracking(True)
    self._idle = True
    # hide until we can test and fix
    #self.startTimer(backend_IdleEvent.milliseconds)
    w, h = self.get_width_height()
    self.resize(w, h)

i.e. the whole init block is copied between the two - however I see no way to avoid it. It could be minimised by a small shared "init()" function but not sure if that improves clarity.

Testing on PySide now and then I'll re-bunch the various commits into a single one (I'm testing on two machines and passing the latest iteration back and forward via github).

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

It works for me with pyqt4, but I get a black window on pysides

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented May 18, 2014

That's reassuring I get the same - but unfortunately no errors! I'm going to pull the latest matplotlib and try gradually applying the changes til it breaks.

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented May 18, 2014

The latest commit fixes the issue with PySide for me (and tidies up a few other things). It is now tested as working for me under:

  • PyQt5 Python2.7 (MacOS X 10.9)
  • PyQt4 Python2.7 (Ubuntu Linux 14.04)
  • PySide Python2.7 (Ubuntu Linux 14.04)
  • PyQt5 Python3 (Ubuntu Linux 14.04)

Can you check on your setup? Thanks

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

with py2.7 it passes the smoke test (I can see a graph) with both pyqt4 as pyside.

What was causing the black window?

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 18, 2014

Never mind, I can read commit messages.

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented May 19, 2014

Good stuff. Should I roll this up into a single commit for possible merge?

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 19, 2014

If you want to squash it down, go ahead, if not it is probably fine.

Please fix the indention issues though.

Implement backend for PyQt5 + modify Qt4 backend to Qt5 structure.
A backend for PyQt5 based on intial work by @badders, modified to fix Qt5 mouse event handling,
then re-structured to implement as a wrapper over the existing Qt4 code. Following discussions on
this pull request:

#2471

The code has been restructured to implement PyQt5 backend as a first-class implementation, with
other Qt backends (PyQt4, PyQt4v2, PySide (Qt v4)) wrapping it and modifying as required. The issues
of objects being moved around in the Qt namespace (many QtGui objects now in QtWidgets) QtWidgets is
simply assigned as a copy of QtGui if not available. This achieves the intended outcome with the
minimum code.

PySide required re-ordering of import on FigureCanvasQTAgg or paintEvent
function would not being called on FigureCanvasQTAggBase resulting in
black window.

A number of indentation, import and other fixes.
@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 19, 2014

@Tillsten Could you test this on windows? I am more worried about regressions in the qt4 functionality.

@Tillsten

This comment has been minimized.

Copy link
Contributor

Tillsten commented May 19, 2014

@tacaswell i will test it as far as i can when i am home (still not able to build mpl with windows), which should be ok for theses changes. but i am quite optimistic, qt is really good at being platform agnostic.

@Tillsten

This comment has been minimized.

Copy link
Contributor

Tillsten commented May 19, 2014

Some encoding issue with one line:

SyntaxError: Non-ASCII character '\xc2' in file C:\Python27\python-2.7.5.amd64\lib\site-packages\matplotlib\backends\backend_qt5agg.py on line 22, but no encoding declared; see http://www.python.org/peps/pep-0263.html for details

It seems the first space in line 22 is some kind of unicode spacing. Using ansi encoding, the line shows up as:
##### Modified Qt5 backend import

from .backend_qt5 import QtGui
from .backend_qt5 import FigureManagerQT
from .backend_qt5 import NavigationToolbar2QT
##### Modified Qt5 backend import

This comment has been minimized.

@tacaswell

tacaswell May 19, 2014

Member

make sure there are no funny characters here, there should be only ascii in these files.

@Tillsten

This comment has been minimized.

Copy link
Contributor

Tillsten commented May 19, 2014

First impression: qt4 seems to work, but _priv_update is not used. See the used draw method in backend_qt5agg (which is used by the qt4 backend too, maybe change the file naming?). I still have to check some event based examples and qt5 under py3.

btw, there are quite a lot of unused imports in backend_qt4agg.

# causes problems with code that uses the result of the
# draw() to update plot elements.
FigureCanvasAgg.draw(self)
self.update()

This comment has been minimized.

@Tillsten

Tillsten May 19, 2014

Contributor

I think this here should be self._priv_update.

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented May 19, 2014

The imports are there to make sure we don't break back compatibility with things that expect to be able to import them from this module. The backends are a bit of a nightmare. If/when we do a 2.0 this is one of the things I would want to aggressively change.

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented Jun 12, 2014

@tacaswell apologies for missing your update. I'm on Python 3.4.0 and checking package versions (dpkg -s python3-pyqt5 libqt5core5a) shows python3-pyqt5 5.2.1 and libqt5core5a 5.2.1+dfsg-1ubuntu14

So I'm a bit stumped why it isn't working for you. Do you get any output at all?
How are you initialising the plot?

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented Jun 12, 2014

I'm not near a computer where I can test this (and won't be for a week and a half). I was testing through ipython + pyplot.

There are no error messages.

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented Jun 12, 2014

Ah, that’s a difference from my approach at least - I was testing from the default Python shell. I’ll try again through IPython and see if I can reproduce.

On 13 June 2014 at 00:24:50, Thomas A Caswell (notifications@github.com) wrote:

I'm not near a computer where I can test this (and won't be for a week and a half). I was testing through ipython + pyplot.

There are no error messages.


Reply to this email directly or view it on GitHub.

Fix error on pyplot initialisation in IPython
There was a bug on the pyplot initialisation that was wrongly looking for qApp in
PyQt5.QtGui rather than PyQt5.QtWidgets. I've updated the init to do this
correctly, which hopefully fixes the bug reported by @tacaswell
@ezust

This comment has been minimized.

Copy link

ezust commented Jun 24, 2014

Has this been pushed to master yet?
I cloned it but I'm having trouble finding any refs to 3072 in the logs.
I'm trying to test this on windows but am having some issues. First,
if I try to run python setup.py, it says:
qt4agg: no [PyQt4 not found] and doesn't look for PyQt5.
I am using Windows 7, Windows SDK 7.1, Python 3.3 x64, PyQt 5.2.1.
Does this matter?

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented Jun 24, 2014

No @ezust this hasn't been merged yet and it's in a separate branch backend-pyqt5. You can certainly try cloning the branch and see if that works (it should). Unfortunately I've no experience with building on Windows so can't help you there, sorry.

mfitzp added a commit to pathomx/pathomx that referenced this pull request Jun 25, 2014

Implements a fix for Qt4 and matplotlib by using the new PyQt4 backen…
…d in matplotlib (branch)

Previously the Qt5 interface was managed by backend_qt5 a temporary
interface that formed the basis of a complete backend implementation
for matplotlib. This has been commited and a PR submitted for inclusion
in matplotlib v1.4.0.

This is available here:
matplotlib/matplotlib#3072

Clone and install from that repo using `pip install .`
@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented Jun 26, 2014

@mfitzp The patch does not fix it for me. I only have issues when the %matplotlib magic is turned on. In either vanilla or ipython with out %matplotlib it works with plt.ion() both on and off. Interestingly, I also can not close the plot windows with the magic turned on. I suspect that this is a bug in ipython, not on our side.

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented Jun 27, 2014

After poking a bit, I am pretty sure this is an issue with the ipython magics. There is some logic in there to explicitly do the mapping between what we call backends and how they keep track of gui frameworks.

@tacaswell tacaswell merged commit 3779fab into matplotlib:master Jun 27, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build could not complete due to an error
Details
@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented Jun 27, 2014

I cleaned up some pep8 violations and merged this into master. I will create an issue with ipython re the magics and create a separate PR dealing with whats_new.rst

@mfitzp

This comment has been minimized.

Copy link
Member Author

mfitzp commented Jun 28, 2014

Great news, thanks for all the guidance on getting this good to go.

@tacaswell

This comment has been minimized.

Copy link
Member

tacaswell commented Jun 28, 2014

Thanks for doing the work to make it happen.

@badders

This comment has been minimized.

Copy link

badders commented Jun 29, 2014

Glad to see this is merged, looking forward to the 1.4.0 release more than ever

@mfitzp mfitzp deleted the mfitzp:backend-pyqt5 branch Sep 4, 2014

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 10, 2015

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 10, 2015

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Feb 19, 2015

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

Conflicts:
	doc/api/api_changes/code_removal.rst

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Apr 19, 2015

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

Conflicts:
	doc/api/api_changes/code_removal.rst

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Apr 19, 2015

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

Conflicts:
	doc/api/api_changes/code_removal.rst

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Sep 16, 2016

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

Conflicts:
	doc/api/api_changes/code_removal.rst

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Sep 17, 2016

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Oct 19, 2016

MNT : remove qt4_compat.py
Deprecated in matplotlib#3174 / f896381

Fall out from the Qt5 upgrade PR matplotlib#3072 / b0dc1d5

@QuLogic QuLogic removed the Needs review label Nov 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.