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

bbox_inches="tight" support for *all* figure artists. #1448

Merged
merged 2 commits into from Mar 31, 2013

Conversation

pelson
Copy link
Member

@pelson pelson commented Nov 3, 2012

This is a first pass at making bbox_inches="tight" work for anything you can throw at a figure.

I think @leejjoon implemented this functionality originally, so it would be great to get some feedback from him as to whether this is a good approach or not.

My motivation for this has been seeing all of the PRs/issues that have gone by lately relating to bbox_inches="tight", most of these have come as a result of iPython defaulting to using this setting (@fperez may be interested in this PR therefore), hopefully this should resolve all outstanding issues on that front (a lot of them have actually already been fixed with bespoke solutions, but inevitably there will be other cases which have not been specifically targeted, hence this PR).

Personally, I still feel that bbox_inches="tight" is an ingenious hack which I would sooner see done better by a geometry manager (#1109), but in lieu of that functionality, getting this right is a worthy pragmatic short-to-medium term goal.

Thanks for your time,

@pelson
Copy link
Member Author

pelson commented Nov 3, 2012

Please note: I am working on my mac and have a couple of concerns about the fonts of the result images (that may well be unfounded). I won't be able to re-generate them with a more reliable freetype until Monday morning.

@@ -14,4 +14,4 @@ install:
script:
- mkdir ../foo
- cd ../foo
- python ../matplotlib/tests.py
- python ../matplotlib/tests.py -sv
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This means we can see if any of the tests are producing standard out (if you're prepared to look at the travis log), and to determine easily which test is producing it.

@jenshnielsen
Copy link
Member

Looks good to me. How about getting rid of the text in the table to make the test less likely to fail with different
freetype versions. Something like the following

import matplotlib.text as text
data = [[  66386,  174296,   75131,  577908,   32015],
        [  58230,  381139,   78045,   99308,  160454],
        [  89135,   80552,  152558,  497981,  603535],
        [  78415,   81858,  150656,  193263,   69638],
        [ 139361,  331509,  343164,  781380,   52269]]

colLabels = ('Freeze', 'Wind', 'Flood', 'Quake', 'Hail')
rowLabels = ['%d year' % x for x in (100, 50, 20, 10, 5)]
rows = len(data)
ind = np.arange(len(colLabels)) + 0.3  # the x locations for the groups
cellText = []
width = 0.4     # the width of the bars
yoff = np.array([0.0] * len(colLabels))
cols = ['r', 'g', 'b', 'k', 'y']
cols2 = cols2 = 5*[cols]
# the bottom values for stacked bar chart
fig, ax = plt.subplots(1,1)
for row in xrange(rows):
    plt.bar(ind, data[row], width, bottom=yoff)
    yoff = yoff + data[row]
    cellText.append(['%1.1f' % (x/1000.0) for x in yoff])
plt.xticks([])
plt.legend(['1', '2', '3', '4', '5'], loc = (1.2, 0.2))
# Add a table at the bottom of the axes
cellText.reverse()
the_table = plt.table(loc='bottom',rowColours=cols, colColours=cols, cellColours=cols2)

jenshnielsen added a commit to jenshnielsen/matplotlib that referenced this pull request Nov 21, 2012
@pelson
Copy link
Member Author

pelson commented Jan 17, 2013

@mdboom - would you mind having a look over this? If you'd rather not have this PR merged, then I won't bother rebasing and bringing it up to speed, but if you think its worthwhile I will.

Cheers,

box at 0, 0.
"""
return Bbox([[0, 0], [0, 0]])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a subclass does not override this method. Will bbox_inches='tight' result in a figure that always contains the point (0, 0)? If so, would it be better to raise NotImplementedError instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bboxes with a width or height of 0 are not included in the final Bbox calculation, so in truth, the Bbox could be based anywhere.

@dmcdougall
Copy link
Member

I would like this to go in v1.3, it would be a lovely enhancement. Thank you for the hard work you've put into this, @pelson.

If there is consensus to include this, this PR needs a CHANGELOG a entry. A nice example would be nice, too. I like that you have tested this well! Good job.

@ghost ghost assigned dmcdougall Jan 18, 2013
@pelson
Copy link
Member Author

pelson commented Jan 18, 2013

Thanks Damon. I'll rebase and address your comments subsequently.

@efiring
Copy link
Member

efiring commented Feb 18, 2013

@pelson, it looks like it would be nice to get this merged soon. I agree with your sentiments expressed in your opening comment.

@dmcdougall
Copy link
Member

There were some python 2.x build errors. I've restarted the build.

dmcdougall added a commit that referenced this pull request Mar 31, 2013
```bbox_inches="tight"``` support for *all* figure artists.
@dmcdougall dmcdougall merged commit 8605579 into matplotlib:master Mar 31, 2013
jenshnielsen added a commit to jenshnielsen/matplotlib that referenced this pull request Jun 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants