FIX: use locators in adjust_bbox #25499

rcomer · 2023-03-18T18:33:30Z

PR Summary

Fixes #22625. The problem was that _ColorbarAxesLocator was not used to set the aspect ratio before adjust_bbox stripped it off. A simpler but presumably less efficient fix might be to trigger a draw before calling adjust_bbox. We already do that if bbox_inches == "tight".

matplotlib/lib/matplotlib/backend_bases.py

Lines 2358 to 2361 in 78bf53c

    
           if layout_engine is not None or bbox_inches == "tight": 
        
               # we need to trigger a draw before printing to make sure 
        
               # CL works.  "tight" also needs a draw to get the right 
        
               # locations:

PR Checklist

Documentation and Tests

Has pytest style unit tests (and pytest passes)
[N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).
[N/A] New plotting related features are documented with examples.

Release Notes

[N/A] New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
[N/A] API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
[N/A] Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

greglucas · 2023-03-19T02:34:04Z

lib/matplotlib/tests/test_figure.py

+    fig, ax = plt.subplots()
+    pc = ax.pcolormesh(np.random.randn(2, 2))
+    cbar = fig.colorbar(pc, aspect=40)
+    fig.savefig(tmp_path / 'foo.png', bbox_inches=mpl.transforms.Bbox([[0, 0], [4, 4]]))


Suggested change

fig.savefig(tmp_path / 'foo.png', bbox_inches=mpl.transforms.Bbox([[0, 0], [4, 4]]))

fig.savefig(io.BytesIO(), bbox_inches=mpl.transforms.Bbox([[0, 0], [4, 4]]))

Thanks, I didn't know you could do this but I see it is the convention through this module. So I took the liberty of also updating the test I added at #24971.

It is likely faster to stay in memory when saving if possible rather than thrashing the hard drive, which I think is why this would be preferred.

greglucas · 2023-03-19T02:35:43Z

lib/matplotlib/tests/test_figure.py

+    cbar = fig.colorbar(pc, aspect=40)
+    fig.savefig(tmp_path / 'foo.png', bbox_inches=mpl.transforms.Bbox([[0, 0], [4, 4]]))
+
+    # Check that an aspect ratio has been applied.


Are you able to assert against the aspect ratio too?

I couldn't figure out how to check the aspect ratio directly. get_aspect() returns "auto" before and after save. get_box_aspect() returns 40 before and after save. Is there something else I've missed?

I didn't know either, I just figured since it was the aspect that was wrong that is better to test against, so figured I would ask if you tried :) but colorbars are never straightforward and there is still an open issue with the get/set aspect on them. #22103

Although here it could also be that the aspect is only changed in the setattr context manager while saving the figure, so you don't get it except while in that context manager?

This still looks good to me!

lumberbot-app · 2023-03-20T21:02:17Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v3.7.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 8ef978d3c3c9461cacb69bb222955c436ad65088

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #25499: FIX: use locators in adjust_bbox'

Push to a named branch:

git push YOURFORK v3.7.x:auto-backport-of-pr-25499-on-v3.7.x

Create a PR against branch v3.7.x, I would have named this PR:

"Backport PR #25499 on branch v3.7.x (FIX: use locators in adjust_bbox)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

Conflict was caused by the new test being right above the test from FIX: use locators in adjust_bbox (cherry picked from commit 8ef978d)

Conflict was caused by the new test being right above the test from matplotlib#24816, which is not in the v3.7.x branch. FIX: use locators in adjust_bbox (cherry picked from commit 8ef978d)

Backport PR #25499: FIX: use locators in adjust_bbox

rcomer added this to the v3.7.2 milestone Mar 18, 2023

rcomer added the PR: bugfix Pull requests that fix identified bugs label Mar 18, 2023

rcomer force-pushed the colorbar-bbox_inches branch from f0db686 to 9df8594 Compare March 18, 2023 18:36

greglucas approved these changes Mar 19, 2023

View reviewed changes

FIX: use locators in adjust_bbox

50aa98a

rcomer force-pushed the colorbar-bbox_inches branch from 9df8594 to 50aa98a Compare March 19, 2023 19:22

oscargus merged commit 8ef978d into matplotlib:main Mar 20, 2023

lumberbot-app bot added the Still Needs Manual Backport label Mar 20, 2023

rcomer deleted the colorbar-bbox_inches branch March 20, 2023 21:05

rcomer mentioned this pull request Mar 21, 2023

Backport PR #25499: FIX: use locators in adjust_bbox #25520

Merged

6 tasks

rcomer removed the Still Needs Manual Backport label Mar 21, 2023

ksunden added a commit that referenced this pull request Mar 21, 2023

Merge pull request #25520 from rcomer/backport-25499

e0de606

Backport PR #25499: FIX: use locators in adjust_bbox

rcomer mentioned this pull request Sep 23, 2023

[Bug]: Error while creating inset axes using mpl_toolkits.axes_grid1.inset_locator.inset_axes #26287

Closed

rcomer mentioned this pull request Feb 9, 2024

[Bug]: colorbar doesn't register inset_axis as cax #27763

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: use locators in adjust_bbox #25499

FIX: use locators in adjust_bbox #25499

rcomer commented Mar 18, 2023

greglucas Mar 19, 2023

rcomer Mar 19, 2023 •

edited

Loading

greglucas Mar 19, 2023

greglucas Mar 19, 2023

rcomer Mar 19, 2023 •

edited

Loading

greglucas Mar 19, 2023

lumberbot-app bot commented Mar 20, 2023

	if layout_engine is not None or bbox_inches == "tight":
	# we need to trigger a draw before printing to make sure
	# CL works. "tight" also needs a draw to get the right
	# locations:

	fig.savefig(tmp_path / 'foo.png', bbox_inches=mpl.transforms.Bbox([[0, 0], [4, 4]]))
	fig.savefig(io.BytesIO(), bbox_inches=mpl.transforms.Bbox([[0, 0], [4, 4]]))

FIX: use locators in adjust_bbox #25499

FIX: use locators in adjust_bbox #25499

Conversation

rcomer commented Mar 18, 2023

PR Summary

PR Checklist

greglucas Mar 19, 2023

Choose a reason for hiding this comment

rcomer Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

greglucas Mar 19, 2023

Choose a reason for hiding this comment

greglucas Mar 19, 2023

Choose a reason for hiding this comment

rcomer Mar 19, 2023 • edited Loading

Choose a reason for hiding this comment

greglucas Mar 19, 2023

Choose a reason for hiding this comment

lumberbot-app bot commented Mar 20, 2023

rcomer Mar 19, 2023 •

edited

Loading

rcomer Mar 19, 2023 •

edited

Loading