savefig with bbox_inches='tight' takes account of all text artists #689
Conversation
| @@ -8250,6 +8250,11 @@ def matshow(self, Z, **kwargs): | |||
| integer=True)) | |||
| return im | |||
|
|
|||
| def get_default_bbox_extra_artists(self, renderer): | |||
| bbox_extra_artists = [t for t in self.texts if t.get_visible()] | |||
| return bbox_extra_artists | |||
There was a problem hiding this comment.
Is it really worth creating the extra variable? Why not just write
return [t for t in self.texts if t.get_visible()]
There was a problem hiding this comment.
I often like the extra named variable before a return so when I encounter problems down the road I can %debug and get in there and explore more efficiently.
|
Thanks! Minor review comments inline, but I can confirm that it fixes the problem both with simple plots and with basemap. Both examples shown in #688 now render correctly. I imagine this will probably cause test breakages for any images that were rendered with 'tight' (if there were any in the test suite), as their layout will change. But the behavior now looks much better! |
|
I just tested this and it looks good. My only comment is that this looks like a bugfix to me. Would it be too difficult for you @leejjoon to modify the pull request to be against v1.1.x rather than master so it makes it into the upcoming bugfix release? |
|
I opened a new PR #739, which is against v1.1.x. I'm closing this PR. |
I have added Figure.get_default_bbox_extra_artists and Axes.get_default_bbox_extra_artists methods. And if bbox_extra_artists is not set for the savefig command, the return value of Figure.get_default_bbox_extra_artists will be used instead.
By default, Figure.get_default_bbox_extra_artists adds all the visible text artists.
I think the default behavior will resolve the issues raised in #688 (although I have not tested this with Basemap).
Note, that the default behavior may result in an incorrect bbox if texts are clipped.
Any feedback will be welcomed.