MNT: Remove cached renderer from figure #23202
Conversation
| """ | ||
| if self.figure._cachedRenderer is None: |
There was a problem hiding this comment.
On one hand, this call making a new renderer would be odd, my understanding of why this exists is a helper specifcally for managing blitting where you can rely on the render not only existing, but also having already had everything else of interest rasterized. By caching the renderer on the figure we are saying "yes, this figure was rendered with this renderer so we are sure that it is holding the state of everything else, also you can use it to draw the given artist". If we cache / create the renderer on the canvas we only get "this renderer will work".
However, that is a narrow case and while I can think of stories of how this would break someone ("catch the AttirbuteError to know we need to re-render the whole figure"....but for that to make sense we would also have to be sure that we were properly invalidating the cache (by setting it to None) when we resize the Figure).
Which is a very long way of saying that I am 👍🏻 on this change (despite some risk), will need an API change note.
There was a problem hiding this comment.
Yes, I agree with all of that. I was a bit surprised that there wasn't more code churn in this PR which does make me a bit hesitant about what the edge-cases I'm missing are...
One thing I thought about is that we have a get_renderer() method on Agg/Cairo Canvas' now, but not on PDF, but the PDF requires a file handle so it isn't as straightforward to just point to a renderer. I was curious if that was perhaps the idea behind the figure caching in the first place... But, surprisingly it didn't affect any of the PDF tests.
There was a problem hiding this comment.
The pdf and svg renderer's work in a streaming mode so going back to one only makes limited sense in general and makes no sense in this case.
4f2cfb1 to
d14c351
Compare
d14c351 to
83381a2
Compare
83381a2 to
7bb5b21
Compare
Let the canvas decide whether to cache the renderer or not and request the renderer from the canvas. The canvas can either return a cached version or a new one.
7bb5b21 to
333af74
Compare
PR Summary
Let the canvas decide whether to cache the renderer or not and let the figure request the renderer from the canvas. The canvas can either return a cached version or a new one.
This is a proof-of-concept after the discussion on the dev call last week to see if we can move the cache from the figure-level to the canvas-level.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).