use Qt window title as default savefig filename #908

mspacek · 2012-05-29T02:13:07Z

When in IPython, I tend to set my figure window title to the last command entered in IPython, ostensibly the command used to generate the figure. When I save the figure, I like to use the same string for the filename, but I've always had to punch that in manually. This patch automates this, but only for the Qt backend, since I'm not familiar with any others. For the typical figure where the user doesn't change the window title, the default filename is now the slightly more descriptive "Figure_x" instead of just "image".

pelson · 2012-05-29T11:13:25Z

A very pertinent mailinglist post: http://old.nabble.com/How-to-give-a-name-to-a-figure--td19498850.html .

I think this pull request should include the definition of an interface to get the window title (as suggested in the mailing list) in a general way in the backend_bases.py, and that your changes make qt the first implementer of the new interface (hence, other backends could then follow suit in time).

pelson · 2012-05-29T11:14:25Z

lib/matplotlib/backends/backend_qt.py

@@ -431,8 +431,10 @@ def save_figure(self, *args):
        sorted_filetypes = filetypes.items()
        sorted_filetypes.sort()
        default_filetype = self.canvas.get_default_filetype()
+        default_filename = self.canvas.window().windowTitle() or 'image'
+        default_filename.replace(' ', '_')


Careful here. Replace is not done in place:

>>> a = 'foo bar' >>> a.replace(' ', '_') 'foo_bar' >>> a 'foo bar'

mspacek · 2012-05-30T01:18:03Z

Whoops, I've made that string in-place error before. Strange thing was it didn't show up in testing. That was because it was actually a QString, for which replace() is in-place apparently :)

I agree, there should be a canvas.get_window_title() method. Should be easy enough to add, at least for qt. I'll get to it shortly.

pelson · 2012-05-30T09:53:44Z

That was because it was actually a QString, for which replace() is in-place apparently :)

Ouch. Thanks for the clarification.

mspacek · 2012-05-30T21:25:29Z

OK, FigureCanvasBase and FigureManagerBase now both have a get_window_title() method. The qt4 backend's FigureCanvasQT overrides it. I've left it undefined for the qt backend, since I don't really know how to get the window title (I presume the qt backend is for qt3), and a quick google search didn't bring up a caption() or getCaption() method analogous to qt3's setCaption() method.

My only concern is that perhaps get_window_title() and set_window_title() should raise NotImplementedErrors in FigureCanvasBase, instead of just using pass. That way, users won't be confused by silent errors on backends for which these methods aren't defined.

mdboom · 2012-05-30T22:05:26Z

Yes -- I think the base class should raise NotImplementedError. It would be great to implement this in at least the major GUI backends (wx, gtk, gtk3, tk, qt4) if possible -- historically, the backends tended to lose feature parity when we don't take care of these things right away. I can probably find some time in the next few days to look at that and extend this PR if you don't have a chance.

mspacek · 2012-05-30T22:15:03Z

OK, done. Sure, if you (or anyone else) with access to the other backends could implement them, that would be great.

pelson · 2012-05-31T08:40:50Z

@mspacek I have sent you a pull request, against this branch, which implements the title in the remaining backends. I have been able to test on a Linux (standard Ubuntu 12.04) machine.

pelson · 2012-06-01T12:08:41Z

lib/matplotlib/backend_bases.py

+        a default filename.
+        """
+        default_filename = self.get_window_title() or 'image'
+        default_filename = default_filename.replace(' ', '_')


As you probably guessed, I would prefer the filename to be lower case for the same reason that the spaces have been removed. Having said that, I'm not overly fussed if you would prefer to keep mixed case.

I think I agree with the lowercase thing. It's more of a gut feeling though -- and part of me feels like it should be lower case on Unix and mixed case on Windows, but that's probably a bad idea ;)

I guess I disagree with going lowercase because it's a loss of information, but I'm outnumbered, so I'll change it back :)

mdboom · 2012-06-01T13:53:25Z

This looks great. Just tested all GUI backends on Linux and seems to work as advertised.

pelson · 2012-06-03T20:12:41Z

lib/matplotlib/backend_bases.py

    def set_window_title(self, title):
        """
        Set the title text of the window containing the figure.  Note that
        this has no effect if there is no window (eg, a PS backend).
        """
-        pass
+        raise NotImplementedError


One thing I am not able to test is the impact of this line. For instance, does the MacOS backend blow up now before any plot is produced? Are there other backends which we don't have sight of which will have this method called, with the obvious impact of having an unexpected exception raised?

mspacek · 2012-06-03T22:09:34Z

@pelson, I guess the point is we want an exception to be raised, so that it doesn't fail silently. And then if reports of failure come back, they can be dealt with as needed?

It seems there isn't a get_window_title or set_window_title in FigureManagerMac. Maybe I should add them both and print a warning? I suppose set_window_title was never implemented, and I don't see it hidden anywhere in _macosx.FigureManager in _macosx.m.

mdboom · 2012-06-04T14:20:50Z

I think @pelson raises a good point about raising NotImplementedError. Maybe get_window_title should instead return some reasonable default in the base class so backends don't add them will continue to work -- with effort of course being put into bringing all the backends in line. I'm unfortunately not a regular Mac user (I have a shared work machine I can fire up on occasion). We should try to get the Mac OS-X backend updated if possible. (It's always a tricky one to get done, since it's the only non-cross-platform backend).

mspacek · 2012-06-04T20:21:32Z

OK. What would be a reasonable default in the base class? A warning? Or should I just restore it to pass?

mdboom · 2012-06-04T20:23:02Z

I think get_window_title should return "image" to maintain the old behavior, which was to use "image.png" as the default filename. set_window_title should probably emit a warning, but otherwise do nothing. That's just my 2c -- there might be an even better way.

mspacek · 2012-06-04T20:31:34Z

Returning "image" on get_window_title in the base class seems reasonable to me.

The other thing to consider is that we probably want to keep the previous behaviour of nothing happening when set_window_title is called on a non-GUI backend, which is sort of a vote for calling pass instead of raising a warning. I suppose there should really be a subclass for non-GUI backends that would override the warning in the base class with a pass.

mdboom · 2012-06-04T21:41:15Z

Good point about the warning. Let's just make it "pass".

Anyone have OS-X and want to tackle implementing this in the mac osx backend?

jkseppan · 2012-06-09T08:02:25Z

There's a (very lightly tested) attempt for OSX in jkseppan:default_filename: jkseppan@8046c39

pelson · 2012-06-09T08:43:09Z

@jkseppan: Great stuff! What setup have you been able to test it on?

jkseppan · 2012-06-09T10:11:43Z

(I updated the commit: jkseppan@08dd29. I submitted it as a pull request to your branch, so you can pull it and this request should automatically update.)

Mac OS X version 10.6.8, Python 2.7 installed from a python.org package:

Python 2.7.2 (v2.7.2:8527427914a2, Jun 11 2011, 15:22:34)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin

As a test, I run:

from matplotlib import pyplot
fig=pyplot.figure()
fig.canvas.set_window_title(u'Hääyöaie')
pyplot.show()

and then click on the save icon. The save window now defaults to "hääyöaie.png" as the filename (though the extension is hidden by default).

mspacek · 2012-06-09T21:31:20Z

Great, thanks for that! I don't understand much in the .m file, but trust that it works.

mdboom · 2012-06-11T12:22:49Z

Great! As this now includes all of the major backends, I think this is probably ready to merge. @mspacek: Any remaining issues you're aware of? Can you do a fresh rebase on master?

pelson · 2012-06-11T12:45:26Z

@mdboom: We've not had any confirmation that this works on windows yet - but we might just have to bite the bullet and merge.
@jkseppan: Were you able to test any of the other backends on your Mac?

jkseppan · 2012-06-11T15:00:27Z

Not so far, because I had trouble installing the various backend requirements on a Mac - but I'm installing some of them via MacPorts now, so I should get to testing the backends soon.

jkseppan · 2012-06-11T17:59:04Z

I was able to test with TkAgg and GtkAgg (Python 2.7.3 installed via MacPorts). Both work; in GtkAgg I can't set the title to a unicode string.

MacPorts is still installing the dependencies for Qt, but I couldn't get WxPython to work via MacPorts.

jkseppan · 2012-06-11T18:55:24Z

With the Qt4Agg backend, the save dialog simply does not appear (Python 2.7.3 via MacPorts).

mdboom · 2012-06-11T18:59:56Z

@jkseppan: Interesting. Is the Qt4Agg problem specific to this branch, or is that true on master as well?

mspacek · 2012-06-11T21:21:45Z

@mdboom, nope, no issues that I know of, at least on Qt4Agg in Linux. I hope I've rebased correctly.

jkseppan · 2012-06-12T04:07:06Z

@mdboom: The same problem occurs on latest master without this change. Getting the dependencies right on OSX is notoriously difficult, so I suspect I have some problem with how I've installed Qt.

pelson · 2012-06-13T14:35:33Z

@jkseppan: Thanks for all your work on this, its been really valuable to test at least one other OS. The Qt issue sounds like a separate issue to this PR, so unless we can get a windows tester on board, this change has my +1.

pelson · 2012-06-15T12:38:06Z

@mdboom & @jkseppan: What do you think? Shall we go for the merge?

@mdboom: I assume there is no current means of writing a unit test for this functionality?

jkseppan · 2012-06-15T12:59:32Z

I agree, go ahead and merge.

jkseppan · 2012-07-02T08:17:37Z

Update: using homebrew, I have been able to test on OSX 10.6, Python 2.7.3, with the following backends:

try_gtk.py:matplotlib.use('GtkAgg')
try_osx.py:matplotlib.use('MacOSX')
try_qt.py:matplotlib.use('Qt4Agg')
try_tk.py:matplotlib.use('TkAgg')
try_wx.py:matplotlib.use('Wx')

Out of these, only Wx fails (so does WxAgg). I don't see a toolbar, and when I press the s key to save, I get a NotImplementedError. I wonder if I'm doing something wrong?

In any case, I'm still in favor of merging.

backend WX version 2.9.3.1
/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/backends/backend_wx.py:1412: wxPyDeprecationWarning: Using deprecated class.

Traceback (most recent call last):
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/backends/backend_wx.py", line 1264, in _onKeyDown
    FigureCanvasBase.key_press_event(self, key, guiEvent=evt)
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/backend_bases.py", line 1608, in key_press_event
    self.callbacks.process(s, event)
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/cbook.py", line 312, in process
    proxy(*args, **kwargs)
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/cbook.py", line 239, in __call__
    return mtd(*args, **kwargs)
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/backend_bases.py", line 2418, in key_press
    key_press_handler(event, self.canvas, self.canvas.toolbar)
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/backend_bases.py", line 2336, in key_press_handler
    toolbar.save_figure()
  File "/opt/homebrew/Cellar/python/2.7.3/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.2.x-py2.7-macosx-10.6-x86_64.egg/matplotlib/backend_bases.py", line 2921, in save_figure
    raise NotImplementedError

pelson · 2012-07-05T21:02:34Z

@mspacek: Would you mind re-basing this PR, then I will merge.

…ig filename revert prior changes to qt backend, since I can't test them

…w_title methods

restore enforcing lowercase in get_default_filename

mspacek · 2012-07-05T21:39:02Z

OK, done. Again, I hope I've rebased correctly. For my own reference, this is roughly what I did:

git co default_filename
git pull origin/master
git rebase origin/master
git push mspacek --force

I noticed that there are two save figure buttons on the figure toolbar now. This must be something new from master.

pelson · 2012-07-06T20:07:24Z

OK, done. Again, I hope I've rebased correctly.

Thanks @mspacek. This is a nice feature. Thanks for all your work on it. Will merge in 24 hrs.

mspacek · 2012-07-08T21:30:37Z

You're welcome!

use window title as default savefig filename

pelson reviewed May 29, 2012
View reviewed changes

pelson reviewed Jun 1, 2012
View reviewed changes

pelson reviewed Jun 3, 2012
View reviewed changes

pelson mentioned this pull request Jun 5, 2012

Customization of NavigationToolbar #348

Closed

mspacek and others added 9 commits July 5, 2012 14:25

use Qt window title as default savefig filename

3983e93

convert QString to string, and fix string.replace() oops

16dd729

in qt4 backend, define get_window_title and call it for default savef…

395acc1

…ig filename revert prior changes to qt backend, since I can't test them

raise NotImplementedError instead of pass for FigureManagerBase windo…

88dbef7

…w_title methods

Supported & tested on wx, gtk, gtk3, qt3, tk.

41aa505

removed lower() on the filename

f60f942

fix qt4 not using get_default_filename in save_figure

c090a9c

restore enforcing lowercase in get_default_filename

maintain compatibility with unimplemented backends

3b19a15

Add window title management to MacOSX backend

23deb00

pelson added a commit that referenced this pull request Jul 9, 2012

Merge pull request #908 from mspacek/default_filename

4f75af3

use window title as default savefig filename

pelson merged commit 4f75af3 into matplotlib:master Jul 9, 2012

efiring mentioned this pull request Jan 14, 2017

get_default_filename modifies user-supplied title for save dialog #7824

Closed

Uh oh!

use Qt window title as default savefig filename #908

use Qt window title as default savefig filename #908

Uh oh!

Conversation

mspacek commented May 29, 2012

Uh oh!

pelson commented May 29, 2012

Uh oh!

pelson May 29, 2012

Choose a reason for hiding this comment

Uh oh!

mspacek commented May 30, 2012

Uh oh!

pelson commented May 30, 2012

Uh oh!

mspacek commented May 30, 2012

Uh oh!

mdboom commented May 30, 2012

Uh oh!

mspacek commented May 30, 2012

Uh oh!

pelson commented May 31, 2012

Uh oh!

pelson Jun 1, 2012

Choose a reason for hiding this comment

Uh oh!

mdboom Jun 1, 2012

Choose a reason for hiding this comment

Uh oh!

mspacek Jun 1, 2012

Choose a reason for hiding this comment

Uh oh!

mdboom commented Jun 1, 2012

Uh oh!

pelson Jun 3, 2012

Choose a reason for hiding this comment

Uh oh!

mspacek commented Jun 3, 2012

Uh oh!

mdboom commented Jun 4, 2012

Uh oh!

mspacek commented Jun 4, 2012

Uh oh!

mdboom commented Jun 4, 2012

Uh oh!

mspacek commented Jun 4, 2012

Uh oh!

mdboom commented Jun 4, 2012

Uh oh!

jkseppan commented Jun 9, 2012

Uh oh!

pelson commented Jun 9, 2012

Uh oh!

jkseppan commented Jun 9, 2012

Uh oh!

mspacek commented Jun 9, 2012

Uh oh!

mdboom commented Jun 11, 2012

Uh oh!

pelson commented Jun 11, 2012

Uh oh!

jkseppan commented Jun 11, 2012

Uh oh!

jkseppan commented Jun 11, 2012

Uh oh!

jkseppan commented Jun 11, 2012

Uh oh!

mdboom commented Jun 11, 2012

Uh oh!

mspacek commented Jun 11, 2012

Uh oh!

jkseppan commented Jun 12, 2012

Uh oh!

pelson commented Jun 13, 2012

Uh oh!

pelson commented Jun 15, 2012

Uh oh!

jkseppan commented Jun 15, 2012

Uh oh!