Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

PySide backend support #80

Merged
merged 14 commits into from Jun 16, 2011

Conversation

Projects
None yet
5 participants
Contributor

gstorer commented Apr 11, 2011

Hi,
I've modified the Qt backend to switch between PySide and PyQt using the same environment variable as IPython.

There are a couple of outstanding bugs in PySide that are apparent in matplotlib:
489 - temporary work around in qt.py that can be removed when PySide fixes the bug. Fixed in source.
819 - temporary work around in qt.py that can be removed when PySide fixes the bug. Default file filter isn't set.
736 - Only affects the subplot configuration tool. Fixed in PySide 1.0.1.

I also removed the 'hackish' fix for the bug in early version of PyQt 4.6.x - it could be added to qt.py if it was felt necessary.

I've added a Qt slot to the FigureManagerQT class that passes straight through to the status bar - this seemed to fix a segfault that may or may not be a bug in PySide. I haven't been able to separately reproduce it and PySide isn't really suppose to support the old style slots/signals.

I've modified formlayout to use getColor instead of getRgba as PySide does not have the later function. I also had to alias all of the Qt names used by formlayout as you can't double import things.

Regards,
Gerald.

Contributor

gstorer commented Apr 28, 2011

Updated to remove the fix for PySide bug 489 and added a work around for the PySide removal of support for old style classes.

The only outstanding bug is 819 where the default file filter can't be set.

Contributor

gstorer commented May 2, 2011

There are now no outstanding bugs with PySide that matplotlib exposes (that I am aware of).

@mdboom mdboom and 1 other commented on an outdated diff May 6, 2011

lib/matplotlib/backends/qt.py
@@ -0,0 +1,53 @@
@mdboom

mdboom May 6, 2011

Owner

The name of this file is problematic. It breaks the Qt3 backend (since 'qt' is the name of the PyQt library version 3 -- the backend tries to import the bindings but instead gets this file.) It's also confusing to read "from qt import ..." in the Qt4 backends and not have that refer to the main qt library.

Perhaps it should be called "qt4_compat.py" or something similar?

@gstorer

gstorer May 6, 2011

Contributor

Good point. I just blindly copied the file from IPython but they don't support Qt3.

@mdboom mdboom and 1 other commented on an outdated diff May 6, 2011

lib/matplotlib/backends/backend_qt4.py
- _getSaveFileName = QtGui.QFileDialog.getSaveFileName
-except (AttributeError, KeyError) :
- # call to getapi() can fail in older versions of sip
- # Use the old getSaveFileName()
- _getSaveFileName = QtGui.QFileDialog.getSaveFileName
-
+ raise ImportError("Qt4 backend requires that PyQt4 or PySide is installed.")
+
+if QT_API == QT_API_PYSIDE:
+ class FigureCanvasBase( FigureCanvasBase, object ):
+ pass
+ class NavigationToolbar2( NavigationToolbar2, object ):
+ pass
+ class SubplotTool( SubplotTool, object ):
+ pass
+
@mdboom

mdboom May 6, 2011

Owner

I gather that PySide requires that its class bases are new-style classes? Why don't we just make these bases new-style classes (i.e. make the canonical FigureCanvasBase in backends.py inherit from object)? That doesn't seem to break anything for any of the other GUI backends and removes the need for this kludge.

@gstorer

gstorer May 6, 2011

Contributor

All correct. If it doesn't break anything I'll make the change on Monday.

@mdboom mdboom commented on the diff May 6, 2011

lib/matplotlib/backends/backend_qt4.py
-
-# XXX Hackish fix: There's a bug in PyQt. See this thread for details:
-# http://old.nabble.com/Qt4-backend:-critical-bug-with-PyQt4-v4.6%2B-td26205716.html
-# Once a release of Qt/PyQt is available without the bug, the version check
-# below can be tightened further to only be applied in the necessary versions.
-if QtCore.PYQT_VERSION_STR.startswith('4.6'):
- class FigureWindow(QtGui.QMainWindow):
- def __init__(self):
- super(FigureWindow, self).__init__()
- def closeEvent(self, event):
- super(FigureWindow, self).closeEvent(event)
- self.emit(QtCore.SIGNAL('destroyed()'))
-else:
- FigureWindow = QtGui.QMainWindow
-# /end pyqt hackish bugfix
-
@mdboom

mdboom May 6, 2011

Owner

Does this drop support for PyQt 4.6 and earlier? At my institution, we are still standardized on RHEL5 which comes with PyQt 4.2. We should support at least that far back.

@gstorer

gstorer May 6, 2011

Contributor

As best I can tell this only has an impact on early versions of 4.6.x. The fix is certainly only applied to PyQt 4.6.x and the bug was apparently fixed in 4.6.2. Even if you're stuck with 4.6 I would've thought you'd at least have the most recent version of it.

I.e. this will only break support for 4.6.0 and 4.6.1 everything else will remain the same.

@mdboom

mdboom May 6, 2011

Owner

Great. Thanks for the confirmation. I think it's fine to not support obsolete maintenance releases.

@mdboom mdboom commented on an outdated diff May 6, 2011

lib/matplotlib/backends/backend_qt4agg.py
+if QT_API == QT_API_PYSIDE:
+ class FigureCanvasAgg( FigureCanvasAgg, object ):
+ pass
+
@mdboom

mdboom May 6, 2011

Owner

See my note above re: new-style classes.

@mdboom mdboom commented on an outdated diff May 6, 2011

lib/matplotlib/backends/qt.py
@@ -0,0 +1,53 @@
+""" A Qt API selector that can be used to switch between PyQt and PySide.
+"""
+
+import os
+
+# Available APIs.
+QT_API_PYQT = 'pyqt'
+QT_API_PYSIDE = 'pyside'
+
+# Use PyQt by default until PySide is stable.
+QT_API = os.environ.get('QT_API', QT_API_PYQT)
+
@mdboom

mdboom May 6, 2011

Owner

I was wondering if we shouldn't do PySide selection by adding a new backend type (Qt4Agg and PySideAgg would share a lot of code, but would be selected with matplotlib.use). However, then I saw that IPython is also using this method, and it would be nice to be compatible with that. Can we add a comment:

Select PyQt4 or PySide using an environment variable, in the same way IPython does.

Contributor

gstorer commented May 9, 2011

All of mdboom's comments implemented. I also changed backend_version while I was at it to be more consistent with the other backends. That is, a version string representing the GUI toolkit's version rather than something arbitrary.

@leejjoon leejjoon commented on the diff May 9, 2011

lib/matplotlib/backends/backend_qt4agg.py
@@ -91,7 +91,7 @@ class FigureCanvasQTAgg( FigureCanvasQT, FigureCanvasAgg ):
else:
stringBuffer = self.renderer._renderer.tostring_argb()
- qImage = QtGui.QImage(stringBuffer, self.renderer.width,
+ qImage = QtGui.QImage(stringBuffer, self.renderer.width,
@leejjoon

leejjoon May 9, 2011

Contributor

With ubuntu 11.4 (pyside 1.0.1 & qt4 4.7.2), I get a following exception.

TypeError: 'PySide.QtGui.QImage' called with wrong argument types:
PySide.QtGui.QImage(str, int, int, PySide.QtGui.QImage.Format)
Supported signatures:
PySide.QtGui.QImage()
PySide.QtGui.QImage(PySide.QtGui.QImage)
PySide.QtGui.QImage(PySide.QtCore.QSize, PySide.QtGui.QImage.Format)
PySide.QtGui.QImage(QString, str = None)
PySide.QtGui.QImage(int, int, PySide.QtGui.QImage.Format)
PySide.QtGui.QImage(buffer, int, int, PySide.QtGui.QImage.Format)
PySide.QtGui.QImage(buffer, int, int, int, PySide.QtGui.QImage.Format)

Converting stringBuffer to a buffer object explicitly seem to solve the problem (both pyqt and pyside works), but i'm not sure if this behavior will depend on the pyside version.

        qImage = QtGui.QImage(buffer(stringBuffer), int(self.renderer.width), 
                              int(self.renderer.height),

We may need a same change at line 117.
The agg renderer has a "buffer_rgba" method but QImage does not seem to support RGBA format.

-JJ

@gstorer

gstorer May 9, 2011

Contributor

This is a PySide bug fixed in 1.0.2 http://bugs.pyside.org/show_bug.cgi?id=489

@mdboom

mdboom May 9, 2011

Owner

We should probably add a version check against PySide then, since we know 1.0.2 is the minimum workable version.

@leejjoon

leejjoon May 9, 2011

Contributor

Good to know that this is a pyside bug. However, using explicit buffer call does any harm to matplotib w/ pyside v1.0.2?
I could be wrong but my guess is that if we call buffer explicitly, the code will work for both v1.0.2 and v1.0.1?

@gstorer

gstorer May 10, 2011

Contributor

Yes. Wrapping with buffer makes it work in both versions. I was under the impression that mpl wasn't in the business of fixing bugs for specific versions of on specific platforms etc. In any case, there's another bug (819, see my opening comment) that wasn't fixed until 1.0.2 (try saving a figure). There is also an outstanding bug that prevents this backend working on mac (809) that won't be fixed until 1.0.3.

I'd suggest that 1.0.3 should probably be the minimum supported version to keep things simple. In which case this can stay as is. Unless there is a good reason to want to wrap with buffer() in any case?

@mdboom

mdboom May 10, 2011

Owner

In my experience, mpl tries to broadly support as many versions of a library on as many platforms as feasible. I don't know if it's stated explicitly anywhere, but at least in my work environment, it is very important to support rather old and possibly buggy versions of core dependencies (e.g. GUI toolkits) for quite some time. Look at the GTK backend, for example, for a number of version-specific hacks that have been required over time. Eventually the idea is to prune that stuff out -- my own rule of thumb is to support at least as far back as whatever was in the (current release - 2) of RHEL.

In this specific case, however, with PySide being so new and cutting edge to begin with, I think it's ok to require a minimum version and not add hacks for 1.0.x which were fairly ephemeral and not widely-used releases. But the version check is important because user's should understand it's breaking because of an old version of PySide and not some other reason.

@leejjoon

leejjoon May 11, 2011

Contributor

I was inclined to fix the buffer thing since pyside v1.0.1 is packaged for ubuntu, and this simple fix at least makes the backend usable. Anyhow, I agree with Michael and the current approach is okay.

@gstorer

gstorer May 11, 2011

Contributor

Ok, that's good then.

As a side note, I was under the impression that Ubuntu was more or less the development platform for PySide. Every version of PySide is packaged for it within a few hours of release isn't it?

Contributor

gstorer commented May 10, 2011

I've added a version check for 1.0.3 or greater. The PySide nightly builds are on 1.0.3 (http://www.pyside.org/files/nightly/) although you're limited to Windows or something Debian based.

Contributor

gstorer commented May 27, 2011

PySide 1.0.3 has been released. I'll update the dev mailing list to solicit some final testing as packages become available.

Contributor

ddale commented Jun 5, 2011

Is this patch ready for final testing? It would be nice to merge in time for the next release.

Contributor

gstorer commented Jun 7, 2011

Yep. The only thing I'm really waiting on is for someone to tell me there are no show stoppers in OS X. This has been a bit tricky since there isn't a PySide 1.0.3 package for OS X yet. I've asked for it on the PySide mailing list today.

Contributor

leejjoon commented Jun 9, 2011

I don't think we have to wait until we have a bug-free PySide. I'm +1 for pulling this immediately. We may update the version check later if necessary.

Contributor

leejjoon commented Jun 9, 2011

Running this code from command line raises an exception during the exit. This only happens for "pyside". With "pyqt", it runs okay. This is not a show stopper and this can be fixed after the patch is pulled.

import matplotlib.pyplot as plt
fig= plt.figure(1)
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/usr/lib/python2.6/atexit.py", line 24, in _run_exitfuncs
    func(*targs, **kargs)
  File "/home/jjlee/.virtualenvs/mpl-head/lib/python2.6/site-packages/matplotlib/_pylab_helpers.py", line 82, in destroy_all
    manager.destroy()
  File "/home/jjlee/.virtualenvs/mpl-head/lib/python2.6/site-packages/matplotlib/backends/backend_qt4.py", line 373, in destroy
    self._widgetclosed )
RuntimeError: Internal C++ object (PySide.QtGui.QMainWindow) already deleted.
Error in sys.exitfunc:
Traceback (most recent call last):
  File "/usr/lib/python2.6/atexit.py", line 24, in _run_exitfuncs
    func(*targs, **kargs)
  File "/home/jjlee/.virtualenvs/mpl-head/lib/python2.6/site-packages/matplotlib/_pylab_helpers.py", line 82, in destroy_all
    manager.destroy()
  File "/home/jjlee/.virtualenvs/mpl-head/lib/python2.6/site-packages/matplotlib/backends/backend_qt4.py", line 373, in destroy
    self._widgetclosed )
RuntimeError: Internal C++ object (PySide.QtGui.QMainWindow) already deleted.
Contributor

gstorer commented Jun 9, 2011

I actually get a error from both pyqt and pyside on exit...

Contributor

gstorer commented Jun 14, 2011

I don't know what was going on before but I can't get a runtime error at all now. This might be platform specific. I'm mostly using Windows XP at work. At any rate I think this is good enough to be pulled into master.

@ddale ddale added a commit that referenced this pull request Jun 16, 2011

@ddale ddale Merge pull request #80 from gstorer/master
PySide backend support
cf0644a

@ddale ddale merged commit cf0644a into matplotlib:master Jun 16, 2011

@richbwood richbwood referenced this pull request Dec 19, 2012

Closed

test_pcolormesh hangs #1609

@magnunor magnunor pushed a commit to magnunor/matplotlib that referenced this pull request Dec 5, 2013

@francisco-dlp francisco-dlp Fixes #80 efe250d

@QuLogic QuLogic modified the milestones: v1.1.0, v2.0.0 Nov 4, 2015

@fariza fariza added a commit to fariza/matplotlib that referenced this pull request Sep 22, 2016

@fariza fariza Merge pull request #80 from fgoudreault/restructure_dataset_generator
Added the points generator.
6aa72b9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment