Fix Axes3D.get_tightbbox to include axis labels when not for_layout_only#31571
Fix Axes3D.get_tightbbox to include axis labels when not for_layout_only#31571Scolliq wants to merge 1 commit into
Conversation
|
Hello @Scolliq, I see you opened two PRs at the same time. We ask that new contributors not open a second PR until the first is closed Please could you decide which of your PRs you would like to move forward with and close the other for now. |
|
Since your PRs look AI driven, please also familiarise yourself with our policy on that |
|
Apologies did not know about the one per person for the initial. The other has been closed. |
|
Please update the PR summary using our template. Please also post the image that you get from running the example code from the issue with your branch. |
| # Include axis labels when computing the full tight | ||
| # bbox (e.g. savefig(bbox_inches='tight')). | ||
| axis_bb = axis.get_tightbbox( | ||
| renderer, for_layout_only=False) |
There was a problem hiding this comment.
I’m not sure I understand the need for the if-loop here. If we are confident that axis.get_tightbbox takes the for_layout_only keyword, can’t we just pass for_layout_only=for_layout_only in both cases? If we are not confident that axis.get_tightbbox takes the for_layout_only keyword, then we should not pass it in the second case. Note that for_layout_only=False is the default for Matplotlib artists where it is implemented.
There was a problem hiding this comment.
Sorry, I actually think this should go the other way. _get_tightbbox_for_layout_only would originally have been used in case axis.get_tightbbox does not take for_layout_only. Obviously the axises on the Axes3D class do take it, but we guard against the possibility that a user or downstream package makes a subclass with axises that don't take it. So the if-loop was correct, but for_layout_only=False should not be passed in the else branch.
|
Thanks for the review, that's a much cleaner approach, I overcomplicated it with the if/else when just passing \ through directly does the same thing in one line. Will fix |
…_only Axes3D.get_tightbbox was calling _get_tightbbox_for_layout_only for every 3D axis unconditionally, which always excludes axis labels. Fix by passing for_layout_only through to axis.get_tightbbox directly, so the tight-save path (for_layout_only=False) includes labels while the layout engine path (for_layout_only=True) is unchanged. Fixes matplotlib#28117.
Scolliq
force-pushed
the
fix/axes3d-tightbbox-labels
branch
from
d63ec6f to
c62a06f
Compare
|
Hi @Scolliq are you still interested in working on this? |
2 participants
PR summary
Fixes a bug where axis labels (xlabel, ylabel, zlabel) on 3D axes are clipped when saving a figure with
bbox_inches='tight'.Root cause:
Axes3D.get_tightbboxunconditionally calledmartist._get_tightbbox_for_layout_onlyfor every 3D axis, which always passesfor_layout_only=Trueinternally and therefore explicitly excludes axis labels from the returned bounding box. This meant the tight-save path never saw the labels, so they were cropped.Fix: Branch on the
for_layout_onlyargument:True(layout engine): keep using_get_tightbbox_for_layout_onlyso layout calculations are unaffected.False(savefig tight path): callaxis.get_tightbbox(renderer, for_layout_only=False)directly so labels are included.Minimum reproducer:
Output with this branch (
bbox_inches='tight') — all labels fully visible:Closes #28117.
AI Disclosure
I used Claude Code (claude-sonnet-4-6) to help locate the relevant code path (
Axes3D.get_tightbbox→_get_tightbbox_for_layout_only) and to draft the fix. I reviewed the logic myself — tracing howfor_layout_onlypropagates throughget_tightbboxand confirming that the layout engine path should remain unchanged — before committing.PR checklist
test_axes3d_tightbbox_includes_axis_labelsadded)