FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

FIX: pass renderer through adjust_bbox by rcomer · Pull Request #29340 · matplotlib/matplotlib · GitHub

Repository navigation

FIX: pass renderer through adjust_bbox - #29340

Merged
jklymak merged 1 commit into
matplotlib:mainfrom
rcomer:inset_axes-renderer
Dec 18, 2024
Merged

jklymak merged 1 commit into
matplotlib:mainfrom
rcomer:inset_axes-renderer

Conversation

rcomer commented Dec 17, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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:

With my branch:

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:

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

PR checklist

github-actions Bot added topic: geometry manager LayoutEngine, Constrained layout, Tight layout topic: canvas and figure manager labels Dec 17, 2024
# 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)

rcomer Dec 17, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 added the PR: bugfix Pull requests that fix identified bugs label Dec 17, 2024

jklymak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

rcomer commented Dec 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 commented Dec 18, 2024

Copy link
Copy Markdown
Member

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

jklymak merged commit dc63ec7 into matplotlib:main Dec 18, 2024
rcomer deleted the inset_axes-renderer branch December 18, 2024 15:52
QuLogic added this to the v3.11.0 milestone Dec 18, 2024

rcomer commented Dec 28, 2024

Copy link
Copy Markdown
Member Author

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

#29383

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL