FIX: be very paranoid about checking what the current canvas is #25573
Conversation
|
Perhaps the simpler check would be Alternatively, we could additionally record |
I think that would still miss the "middle removed" issue as we have references that skip generations and we should avoid adding more hard-references than we strictly need.... But I will change to |
54db976 to
7100b57
Compare
|
Hmm, we are back to not being careful enough...but only an azure?! |
|
That's a bit fishy, but perhaps just change all the |
a9f4c6c to
db2176f
Compare
lib/matplotlib/widgets.py
Outdated
| @@ -1085,7 +1087,7 @@ def __init__(self, ax, labels, actives=None, *, useblit=True, | |||
|
|
|||
| def _clear(self, event): | |||
| """Internal event handler to clear the buttons.""" | |||
| if self.ignore(event) or self.canvas.is_saving(): | |||
| if self.ignore(event) or self.canvas is None or self.canvas.is_saving(): | |||
There was a problem hiding this comment.
Should the canvas is not None check rather go to a new AxesWidget.ignore? (def AxesWidget.ignore(self, event): return not self.active or self.canvas is None)
Then this change and the one immediately below could go away.
There was a problem hiding this comment.
I agree. This belongs logically in an AxesWidget.ignore().
|
I tried putting the check in Added a new method and a couple of |
lib/matplotlib/tests/test_widgets.py
Outdated
| def test_parent_axes_removal(): | ||
|
|
||
| fig, (ax_radio, ax_checks) = plt.subplots(1, 2) | ||
|
|
||
| radio = widgets.RadioButtons(ax_radio, ['1', '2'], 0) | ||
| checks = widgets.CheckButtons(ax_checks, ['1', '2'], [True, False]) | ||
|
|
||
| ax_checks.remove() | ||
| ax_radio.remove() | ||
| with io.BytesIO() as out: | ||
| fig.savefig(out, format='raw') | ||
|
|
||
| renderer = fig._get_renderer() | ||
| evt = DrawEvent('draw_event', fig.canvas, renderer) | ||
| radio._clear(evt) | ||
| checks._clear(evt) |
There was a problem hiding this comment.
This is obviously some kind of smoke test as it does not contain assertions. A short description would be helpful though.
I understand that removal of the buttons should not brick the figure. But what exactly are we testing with the savefig, and what with the _clear calls? (I assume there are actually two separate tests?
9f1da21 to
5c83d7b
Compare
PR Summary
closes #25572
There are many ways for things to end up in odd states, the goal here is to detect if the the canvas has changed under us when trying to manage the state of widgets.
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst