Skip to content

Add a Figure._get_cachedRenderer() method #21319

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add a Figure._get_cachedRenderer() method #21319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions lib/matplotlib/figure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2376,6 +2376,13 @@ def axes(self):

get_axes = axes.fget

def _get_cachedRenderer(self, error_if_none=True):
# Get the cached renderer, raising an error if it doesn't exist yet
Copy link
Member

Choose a reason for hiding this comment

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

I guess if self._cachedRenderer is None, I'd do self.draw_without_rendering() here and force a renderer (I think).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is probably a behaviour change and outside the scope of this (bugfix) PR? Perhaps not though, I'm not very well acquainted with renderers etc.

Copy link
Member

Choose a reason for hiding this comment

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

We already have a public method https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.get_renderer_cache.html ax.get_render_cache, do we want to prompt this to be public on the Figure too (or deprecate the one on Axes? We do not use it internally, it only shows up where it is defined, in an example, and in the source rst).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, and I'm not entirely sure what the answer is as I'm not too sure why/if a user would want to get the cached renderer.

Copy link
Member

Choose a reason for hiding this comment

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

I think for exactly the reason we are getting it here!

I can see an argument than all of the renderer cache stuff should be private. It is a cache and there is no good reason for the user to be poking around the cache, we want to be able to replace/clear it/etc at will. On the other hand, we expose a couple of public functions that take a renderer as an "optional" (but only because fill it in with the cache) argument. Making renderers is a bit finicky (and if you make it your self no guarantee that it is consistent with anything you care about!).

if error_if_none and self._cachedRenderer is None:
raise RuntimeError("This code can only be used after an "
"initial draw which caches the renderer.")
return self._cachedRenderer

def _get_dpi(self):
return self._dpi

Expand Down Expand Up @@ -2829,10 +2836,7 @@ def draw_artist(self, a):
This method can only be used after an initial draw of the figure,
because that creates and caches the renderer needed here.
"""
if self._cachedRenderer is None:
raise AttributeError("draw_artist can only be used after an "
Copy link
Member

Choose a reason for hiding this comment

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

I suspect that this is raising AttributeError to maintain exact API compatibility with older versions of the code where _cachedRenderer was not an attribute until it was stashed 😱 .

"initial draw which caches the renderer")
a.draw(self._cachedRenderer)
a.draw(self._get_cachedRenderer())

def __getstate__(self):
state = super().__getstate__()
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/legend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ def get_title(self):
def get_window_extent(self, renderer=None):
# docstring inherited
if renderer is None:
renderer = self.figure._cachedRenderer
renderer = self.figure._get_cachedRenderer()
return self._legend_box.get_window_extent(renderer=renderer)

def get_tightbbox(self, renderer):
Expand Down
8 changes: 8 additions & 0 deletions lib/matplotlib/tests/test_legend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,3 +880,11 @@ def test_subfigure_legend():
ax.plot([0, 1], [0, 1], label="line")
leg = subfig.legend()
assert leg.figure is subfig


def test_no_renderer_error():
leg = plt.legend()
with pytest.raises(
RuntimeError,
match='This code can only be used after an initial draw'):
leg.get_window_extent()