Added savefig.bbox option to matplotlibrc #1004

Merged
merged 6 commits into from Jul 23, 2012

Conversation

Projects
None yet
3 participants
Contributor

oxling commented Jul 10, 2012

This is from wishlist issue #988. It allows the user to default the bounding box to 'tight.'

@oxling oxling Added savefig.bbox option to matplotlibrc
Allows the user to set the bounding box to 'tight,' which will override the defaults when saving from the interface.
4047815

@pelson pelson and 1 other commented on an outdated diff Jul 12, 2012

lib/matplotlib/rcsetup.py
@@ -325,6 +325,16 @@ def validate_hinting(s):
validate_movie_frame_fmt = ValidateInStrings('animation.frame_format',
['png', 'jpeg', 'tiff', 'raw', 'rgba'])
+def validate_bbox(s):
+ if type(s) is str:
+ s = s.lower()
+ if s in ('tight'):
+ return s
+ if s in ('auto'):
@pelson

pelson Jul 12, 2012

Member

I don't think this line does as you intended:

>>> 'a' in ('hello world')
False
>>> 'hello' in ('hello world')
True
>>> 'w' in ('hello world')
True

Adding a trailing comma to turn the thing into a tuple was probably your intention:

>>> 'w' in ('hello world', )
False
>>> 'hello world' in ('hello world', )
True
@pelson

pelson Jul 12, 2012

Member

But really, you may as well just use string equality:

if s == 'auto'
@oxling

oxling Jul 13, 2012

Contributor

Thanks for the correction, I'll use string equality.

Member

WeatherGod commented Jul 13, 2012

The current behavior for the savefig function is to save the visible canvas area if "bbox_inches" was not given or was None. This does not change with the new code. However, there is a slight symantical collision here, as it is a general rule that passing None for a kwarg value means "perform the default behavior", which, in turn, typically means, use the rc default. So, if I set "bbox_inches" to "tight", and pass bbox_inches=None to the savefig kwargs, I get a different result than intended. (Note, this is not a new problem. It has been a growing issue with kwarg handling that really needs to be addressed throughout matplotlib).

I also don't like "auto", because there is nothing automatic about it. In other words, when no bbox is specified, it just uses the existing bbox of the figure. Of course this is a minor quible, and I am probably being too pedantic.

Next, having "auto" as a possible value for bbox_inches implies that one could use "auto" for input to savefig(). This might actually be a good thing in that it might help address my first point.

Finally, there are other kwargs that get activated when bbox_inches is "tight". Adding "pad_inches" should be trivial, but I don't see how we could implement bbox_extra_artists. I guess we would just have to do without bbox_extra_artists, but pad_inches should definitely be included.

Contributor

oxling commented Jul 13, 2012

@WeatherGod OK, I made most of your suggested changes. I changed 'auto' to 'default,' added savefig.pad_inches, and changed the behavior so that passing 'None' will use the value set in the defaults.

Whitespace change here

@WeatherGod WeatherGod commented on an outdated diff Jul 13, 2012

lib/matplotlib/backend_bases.py
@@ -2012,6 +2014,9 @@ def print_figure(self, filename, dpi=None, facecolor='w', edgecolor='w',
self.figure.set_edgecolor(edgecolor)
bbox_inches = kwargs.pop("bbox_inches", None)
+ if bbox_inches == None:
@WeatherGod

WeatherGod Jul 13, 2012

Member

Don't use '==' when testing for None. Always use "is" and "is not".

@WeatherGod WeatherGod and 1 other commented on an outdated diff Jul 13, 2012

lib/matplotlib/backend_bases.py
@@ -2052,8 +2057,10 @@ def print_figure(self, filename, dpi=None, facecolor='w', edgecolor='w',
bbox_inches = Bbox.union([bbox_inches, bbox_inches1])
+ pad = kwargs.pop("pad_inches", None)
+ if pad == None:
@WeatherGod

WeatherGod Jul 13, 2012

Member

use "is". Never use '==' for testing for None.

@oxling

oxling Jul 13, 2012

Contributor

Thanks, fixed in 88b3661

Member

WeatherGod commented Jul 13, 2012

I dislike "default" even more than "auto" because which default does it refer to? How about "standard", "keep" or maybe "unchanged"?

Contributor

oxling commented Jul 13, 2012

How about just passing in None instead? That's what the savefig() function expects anyways, and it seems to keep with the same style of passing True or False in the defaults file.

Member

WeatherGod commented Jul 13, 2012

I would rather avoid clouding the distinction between "None" and None. We already have enough problems as is with that. We already do have None's in the rc file, but those mean the string "None" which is entirely different from the python None. In the rc file, we can not distinguish between the two since everything comes in as strings.

Contributor

oxling commented Jul 13, 2012

OK. Changed to 'standard.'

Member

WeatherGod commented Jul 18, 2012

ok, remind me if there were anything left holding up this PR? I think all my qualms have been addressed.

Contributor

oxling commented Jul 18, 2012

I don't think there was.

On Jul 18, 2012, at 12:58 AM, Benjamin Rootreply@reply.github.com wrote:

ok, remind me if there were anything left holding up this PR? I think all my qualms have been addressed.


Reply to this email directly or view it on GitHub:
#1004 (comment)

Member

WeatherGod commented Jul 18, 2012

Oh, right, can you please add a note to docs/users/whats_new.rst?

Member

WeatherGod commented Jul 21, 2012

Be sure to ping me when you make that last change so I can go ahead and merge this in. Thanks for your work!

Contributor

oxling commented Jul 23, 2012

OK, done. Let me me know if I didn't get the formatting in that file right.

Move this line to be below your addition.

Owner

oxling replied Jul 23, 2012

Fixed

Member

WeatherGod commented Jul 23, 2012

Ok, looks good to me. Merging it in now. Thanks!

WeatherGod merged commit 561d0da into matplotlib:master Jul 23, 2012

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