Skip to content

FIX: pass renderer through adjust_bbox #29340

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

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Dec 17, 2024

PR summary

Closes #27763.

I have not fully understood what was going wrong, but I did discover that when fig._get_renderer is called, fig.dpi gets reset to 72. As the renderer had deliberately set it to something else here, this is probably not helpful! Anyway, it is intuitive to me that the renderer used to work out where things are should be the same renderer that does this part of the drawing. Taking a simplified version of the #27763 example,

from mpl_toolkits.axes_grid1.inset_locator import inset_axes
import matplotlib.pyplot as plt
import numpy as np

fig, axes = plt.subplots(1, 1, dpi=100)

x = np.random.randint(100, size=(10, 200))
c = axes.imshow(x, origin='lower', cmap='Reds', aspect='auto')
axins1 = inset_axes(axes,
                    width="10%",
                    height="30%",
                    loc="upper left",
                    bbox_to_anchor = (0.045, 0., 1, 1),
                    bbox_transform = axes.transAxes
                    )

fig.colorbar(c, cax=axins1, orientation="horizontal")

fig.savefig('test_single.pdf', bbox_inches='tight')

With main:
image

With my branch:
image

The alignment is still not perfect (as also shown in the new test image), but it looks identical to what I get with v3.7.1 (before I started breaking things 🐑). I note that several commenters on the issue seemed happy with v3.7.1 behaviour. Also I cannot see any offset if I increase the dpi to 300:

image

I propose to open a new issue to track the remaining small offset.

PR checklist

@github-actions github-actions bot added topic: geometry manager LayoutEngine, Constrained layout, Tight layout topic: canvas and figure manager labels Dec 17, 2024
@@ -2169,7 +2172,7 @@ def print_figure(

# call adjust_bbox to save only the given area
restore_bbox = _tight_bbox.adjust_bbox(
self.figure, bbox_inches, self.figure.canvas.fixed_dpi)
self.figure, bbox_inches, renderer, self.figure.canvas.fixed_dpi)
Copy link
Member Author

@rcomer rcomer Dec 17, 2024

Choose a reason for hiding this comment

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

Passing the renderer here does not affect the target issue but I assume that passing it when we have it is more efficient than potentially calling _get_renderer again.

@rcomer rcomer added the PR: bugfix Pull requests that fix identified bugs label Dec 17, 2024
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Is there a test we can do with the main library, or are the effects too subtle there?

@rcomer
Copy link
Member Author

rcomer commented Dec 18, 2024

Is there a test we can do with the main library, or are the effects too subtle there?

We need an axes that uses a locator that requires a renderer. Do we have one of those in the main library? I checked that Axes.inset_axes' locator works without a renderer.

Also I should have mentioned that this change does nothing for your example at #27763 (comment) (although that one also looks fine if you bump the dpi to 300).

@jklymak
Copy link
Member

jklymak commented Dec 18, 2024

OK< so there is still a small registration issue. Thanks for checking.

@jklymak jklymak merged commit dc63ec7 into matplotlib:main Dec 18, 2024
41 of 42 checks passed
@rcomer rcomer deleted the inset_axes-renderer branch December 18, 2024 15:52
@QuLogic QuLogic added this to the v3.11.0 milestone Dec 18, 2024
@rcomer
Copy link
Member Author

rcomer commented Dec 28, 2024

I propose to open a new issue to track the remaining small offset.

#29383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bugfix Pull requests that fix identified bugs topic: canvas and figure manager topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: colorbar doesn't register inset_axis as cax
4 participants