FIX: do not let no-op monkey patches to renderer leak out#17560
FIX: do not let no-op monkey patches to renderer leak out#17560QuLogic merged 2 commits intomatplotlib:masterfrom
Conversation
tacaswell
force-pushed
the
fix_noop_tight_bbox
branch
from
91e2fd8 to
297cf79
Compare
There was a problem hiding this comment.
Looks fine, but perhaps this should instead be a method (private or public) on the RendererBase class?
def _draw_disabled(self):
return _setattr_cm(**{...})so that you can just write
with renderer._draw_disabled(): ...saving on the duplication? Also this way, at least in theory, Renderer subclasses could customize that behavior as needed (effectively bringing back a dryrun-like functionality (#14134), but on a strictly optional basis).
|
I'm 50/50 on moving it to the base class. On one hand I agree it is much neater. On the other hand, how strongly do we require the renderers to actually be sub-classes? I would say that adding a new method to the semi-public API of the renderer is something we should not do in a patch release and we probably should backport this to 3.2.2 to fix the empty figure bug. |
|
We basically do per #17159. |
|
Will add helper function. |
|
I made it forgiving for now (to backport to 3.2.x) but I'll open a follow-on PR to make it warn for 3.3. |
tacaswell
force-pushed
the
fix_noop_tight_bbox
branch
from
8698d15 to
7a412cd
Compare
anntzer
left a comment
anntzer
left a comment
There was a problem hiding this comment.
doc needs update (so approval conditional on that) but otherwise looks good
Be forgiving about renderer instances that do not inherit from RendereBase.
tacaswell
force-pushed
the
fix_noop_tight_bbox
branch
from
7a412cd to
f777177
Compare
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
… renderer leak out Merge pull request matplotlib#17560 from tacaswell/fix_noop_tight_bbox FIX: do not let no-op monkey patches to renderer leak out Conflicts: lib/matplotlib/backend_bases.py - trivial conflicting imports - did not back port other changes to tight box lib/matplotlib/tight_layout.py - ended up not backporting any of the changes
… renderer leak out Merge pull request matplotlib#17560 from tacaswell/fix_noop_tight_bbox FIX: do not let no-op monkey patches to renderer leak out Conflicts: lib/matplotlib/backend_bases.py - trivial conflicting imports - did not back port other changes to tight box lib/matplotlib/tight_layout.py - ended up not backporting any of the changes
…-v3.2.x Backport PR #17560: FIX: do not let no-op monkey patches to renderer …
3 participants
PR Summary
closes #17542
This issue is that in some cases the no-op monkey patched renderer was getting cached deep in the backend code and the no-op functions were getting used for the actual save which resulted in empty figures. This moves the monkey patching to the call sites so we can use the
_setattr_cmto manage this.PR Checklist