Add a Figure._get_cachedRenderer() method #21319
Conversation
|
Yes I was wondering if it should try and get the rendere from the current figure if there is one in pyplot. |
| @@ -2378,6 +2378,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 | |||
There was a problem hiding this comment.
I guess if self._cachedRenderer is None, I'd do self.draw_without_rendering() here and force a renderer (I think).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!).
dstansby
force-pushed
the
cahced-render
branch
from
41ece96
to
883d2cf
Compare
This comment has been minimized.
This comment has been minimized.
dstansby
force-pushed
the
cahced-render
branch
from
883d2cf
to
64fc6fb
Compare
| @@ -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 " | |||
There was a problem hiding this comment.
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 😱 .
|
I pushed this to 3.6 because I think this is a slightly bigger change than we want to put in a patch release, this is a very old bug, and there is multiple reasonable workarounds. |
|
I think this needs an API change note because the Exception type is changed. We may also want to do class NoRenderer(RuntimeError, AttributeError):
...and raise that instead (back-compatible and a more informative name). |
|
@dstansby popped this to draft until the rebase and API change note are done. I imagine it is mergeable at that point? |
|
I don't have time to pick this up in the short term - if someone else wants to feel free |
|
I think so.... |
PR Summary
Adds
Figure._get_cachedRenderer()which can be used to get the cached renderer, but error if it's not present. This can be used in one existing place, and is also now used in a legend method, which probably fixes #21285PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).