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

FIX: allow non bbox_extra_artists calls #12635

Merged
merged 2 commits into from
Oct 28, 2018

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Oct 26, 2018

PR Summary

Closes #12634

Apparently some axes don't allow accept bbox_extra_artists kwarg. I could not track down how to get the parasite axes' get_tightbbox to accept this kwarg, but this fix seems to work fine.

The offending PR was #12363 where I introduced the bbox_extra_artists as a kwarg. That was probably a mistake because some axes subclasses out there may not impliment this kwarg.

This PR makes that kwarg optional - if axsubclass.get_tightbbox(renderer, bbox_extra_artists=artists) gives a TypeError, then we just call it w/o this kwarg.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant

@jklymak jklymak added this to the v3.0.x milestone Oct 26, 2018
@jklymak jklymak added topic: geometry manager LayoutEngine, Constrained layout, Tight layout topic: mpl_toolkit labels Oct 26, 2018
@jklymak
Copy link
Member Author

jklymak commented Oct 26, 2018

Note I still need to add a test or two. This shouldn’t have passed CI (EDIT: shuld not have passed CI when the original PR went in)...

EDIT: test added!

@jklymak jklymak force-pushed the fix-allow-non-bbox-exgtra-artists branch from 985eae1 to ed621ad Compare October 26, 2018 17:27
@timhoffm timhoffm merged commit 6af7450 into matplotlib:master Oct 28, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 28, 2018
jklymak added a commit that referenced this pull request Oct 29, 2018
…635-on-v3.0.x

Backport PR #12635 on branch v3.0.x (FIX: allow non bbox_extra_artists calls)
@@ -406,3 +406,18 @@ def test_image_grid():
for i in range(4):
grid[i].imshow(im)
grid[i].set_title('test {0}{0}'.format(i))


def test_gettightbbox():
Copy link
Member

Choose a reason for hiding this comment

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

Have you looked at the figure produced in this test? Is it working the way it's supposed to? I get one Axes with one really big Axes overlaid on top of it, but I'm not sure if that's the intended behaviour or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, sorry, its not a very good test. But I'm not sure why axes_grid does that, so that's a different issue. The issue here was that the png being made was not being cropped properly to include the big box when bbox_inches='tight' was used.

@anntzer anntzer mentioned this pull request Apr 28, 2020
6 tasks
@jklymak jklymak deleted the fix-allow-non-bbox-exgtra-artists branch April 28, 2020 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: geometry manager LayoutEngine, Constrained layout, Tight layout topic: mpl_toolkit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

axes_grid1 axes have no keyword argument 'bbox_extra_artists'
4 participants