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

Add default savefig directory #1685

Merged
merged 12 commits into from Feb 17, 2013

Conversation

Projects
None yet
7 participants
Contributor

mspacek commented Jan 19, 2013

A while ago, we added the ability to supply the default filename to the savefig dialog. That was handy. But, I find when I'm generating figures, I'm usually saving them all to the same directory. Having to constantly navigate there from the current working directory that the Python code is running in is very tedious. This PR saves the last used savefig directory, and fills that into the savefig dialog next time you try to save a figure. This is remembered for the duration of the current Python session by updating a savefig.directory key in the rcParams dict. This PR also adds savefig.directory to the matplotlibrc file, so you can control what the initial default is when you try and save a figure for the first time in your Python session.

I've only added this to Qt4 figures, since that's the only GUI backend I ever use. Perhaps other GUI backends should get this too? I'd prefer it if someone could branch my savefig_directory branch and provide that code, since I'm only really familiar with Qt4.

Owner

efiring commented Jan 19, 2013

I haven't looked at the implementation, but this sounds like a highly desirable feature--provided it can be implemented in all major gui backends. I expect it can.

@pelson pelson commented on an outdated diff Jan 21, 2013

lib/matplotlib/backends/backend_qt4.py
@@ -648,6 +650,7 @@ def save_figure(self, *args):
fname = _getSaveFileName(self, "Choose a filename to save to",
start, filters, selectedFilter)
if fname:
+ matplotlib.rcParams['savefig.directory'] = os.path.split(str(fname))[0]
Member

pelson commented Jan 21, 2013

I agree with @efiring - this will need to be done across all (at least the reasonably common) backends.

I would also like to see an option to have it not remember where I last saved my figure, and use the PWD as it used to. (That doesn't necessarily need to be default though).

Contributor

mspacek commented Jan 21, 2013

@pelson, OK, I think I have that working. If the rc dict value is set to a blank string, or if the key is missing, or if the value is empty in the matplotlibrc file, the feature is disabled and the savefig dialog always uses the CWD. But of course, only in qt4.

Member

pelson commented Jan 22, 2013

This looks good to me. Are you in a position to apply this functionality to TkAgg and GtkAgg? I think perhaps other backends could follow in a separate PR as and when users/developers want them.

@mdboom & @efiring - perhaps we should keep a feature table to keep track of what functionality each backend has. This would be useful for other PRs such as the one relating to modifier keys that I implemented & had merged last summer.

Contributor

mspacek commented Jan 22, 2013

OK, here's my stab at adding the savefig.directory feature to tk, gtk, and gtk3. Lightly tested.

@pelson pelson commented on the diff Jan 23, 2013

lib/matplotlib/backends/backend_tkagg.py
)
if fname == "" or fname == ():
return
else:
+ if initialdir == '':
@pelson

pelson Jan 23, 2013

Member

I'm not entirely clear what this if block is for - if the initialdir is '' then is that not what was already in rcParams?

@mspacek

mspacek Jan 23, 2013

Contributor

It's there in case the rcParams['savefig.directory'] key was missing. Maybe the user deleted it from the rcParams dict after initialization by rcsetup.py? Perhaps that's unreasonable to worry about. I have the same if block in the other backends. I could remove it from all backends, and then to be consistent, change all instances of rcParams.get('savefig.directory', '') to just rcParams['savefig.directory'].

Member

pelson commented Jan 23, 2013

I've tested on TkAgg on a 64 bit RHEL 6 machine - it works as expected.

My biggest concern is whether any of the GUI backends already provide this sticky directory behaviour - and if they do, it is likely to be cross session rather than bound to a single mpl session. Now that it is possible to disable this functionality, that behaviour is easily retained, but I wonder if the default behaviour should be the same as it always has been, and those who want the functionality here can opt-in.

Thoughts anybody?

Member

WeatherGod commented Jan 23, 2013

I think in this case, I would want opt-out. On the backends I use
regularly (Tk and Gtk), I don't see any sort of existing sticky directory
behavior, and it has always driven me a little batty, so this feature would
be a welcome surprise to those annoyed by this in past releases. A reason
not to make it opt-in is that it is hard for those who would want this
behavior to discover that an option exists to turn it on. GUI interfaces
are often relegated to the "just put up with it" part of my brain, and so
it is unlikely that I would try and look for it.

Contributor

mspacek commented Jan 23, 2013

I tested each of qt4, tk, gtk, and gtk3 backends before applying the fix, and from what I could tell, none of them had sticky directory behaviour.

Member

pelson commented Jan 23, 2013

Ok. Thanks guys - you've re-assured me on this.

Looking over it, I think it needs an entry in the changelog and I think it then has my 👍. It would be good to get some more eyes on this first though.

Thanks @mspacek!

Owner

mdboom commented Jan 23, 2013

Great idea -- and I agree with this becoming the new default. It just seems so much more sane than the current behavior. I think this should get a short What's New entry as well -- I think it's prominent enough to not only be mentioned in the CHANGELOG.

Contributor

mspacek commented Jan 23, 2013

Great, thanks! I've updated CHANGELOG.

By the way, I only implemented this in NavigationToolbar2. Pretty sure it doesn't work for the old toolbar, though I haven't tested that thing in years...

Member

dmcdougall commented Jan 23, 2013

@mspacek I think the old toolbar was deprecated.

Contributor

mspacek commented Jan 25, 2013

Is there anything else holding this up from being merged? I did a quick check, none of these files have been modified lately, so it should merge smoothly.

@efiring efiring commented on an outdated diff Jan 26, 2013

lib/matplotlib/backends/backend_gtk.py
@@ -1010,8 +1018,7 @@ def save_figure(self, *args):
class FileChooserDialog(gtk.FileChooserDialog):
- """GTK+ 2.4 file selector which remembers the last file/directory
- selected and presents the user with a menu of supported image formats
+ """GTK+ 2.4 file selector which presents the user with a menu of supported image formats
@efiring

efiring Jan 26, 2013

Owner

This line should be split.

Owner

efiring commented Jan 26, 2013

@mdboom requested a "What's new" entry in addition to the CHANGELOG entry.
Also, github is claiming "This pull request cannot be automatically merged." I don't know what is causing the conflict--probably the CHANGELOG. You could try rebasing.

Contributor

mspacek commented Jan 26, 2013

Ah, I didn't realize there's a "what's new" file in the repo. Done, and rebased. Yes, the conflict was in CHANGELOG.

Contributor

iMichka commented Jan 28, 2013

Hi

I just tested this new feature and found a little problem.If the path contains non-ascii characters, you will get an error message due to the str() conversion. You should change the str() to unicode() everywhere :

rcParams['savefig.directory'] = os.path.dirname(unicode(fname))

And in rcsetup.py :

'savefig.directory' : ['~', unicode],

Owner

mdboom commented Jan 28, 2013

@iMichka: Does that actually work on Python 2? It looks like we're reading in the .matplotlibrc without any encoding, so we can't really expect to store unicode in the file. We should probably read it in in utf-8, or read some sort of encoding header in the file (though the former is easier). I'll open a new issue for this.

Contributor

iMichka commented Jan 28, 2013

I tested it with python 2.7 (under OS X 10.7.5, python is from macports). Worked with my changes out of the box.
I don't know if it will work with other platforms ? Perhaps we could use fname = frame.encode('utf-8') (Not sure here because I am not a specialist of those things).

Contributor

mspacek commented Jan 28, 2013

I really don't know anything about unicode. Should I add the proposed changes, or would this be better left to a different PR?

Owner

mdboom commented Jan 28, 2013

I think you can go ahead and make the changes (they can't hurt), but Unicode paths may not work on all platforms/combinations. But we can patch that up later...

Contributor

mspacek commented Jan 28, 2013

OK, done. Seems to work for me on Python 2.7 in Ubuntu 12.10, but I don't have any non-ascii characters in my paths.

Contributor

mspacek commented Feb 1, 2013

@mdboom, sorry to nag, but is there anything else holding up this PR? I just rebased against master again.

@efiring efiring added a commit that referenced this pull request Feb 17, 2013

@efiring efiring Merge pull request #1685 from mspacek/savefig_directory
Add default savefig directory
20cd99c

@efiring efiring merged commit 20cd99c into matplotlib:master Feb 17, 2013

1 check failed

default The Travis build failed
Details
Contributor

mspacek commented Feb 20, 2013

Thanks for the merge!

@mspacek mspacek deleted the mspacek:savefig_directory branch Feb 20, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment