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

Has pytest style unit tests (andpytest passes)
[N/A] Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
[N/A] New plotting related features are documented with examples.

Release Notes

[N/A] New features are marked with a.. versionadded:: directive in the docstring and documented indoc/users/next_whats_new/
[N/A] API changes are marked with a.. versionchanged:: directive in the docstring and documented indoc/api/next_api_changes/
[N/A] Release notes conform with instructions innext_whats_new/README.rst ornext_api_changes/README.rst

tacaswell added this to thev3.7.2 milestone

Mar 29, 2023

timhoffm approved these changes

Mar 29, 2023

View reviewed changes

Copy link

Contributor

anntzer commentedMar 29, 2023

Perhaps the simpler check would beself.canvas is self.canvas.figure.canvas? Note that while figure.canvas can change (hence the introduction of _changed_canvas), I think canvas.figure does not ever change (at least in practice... it may even be possible to spec that), so the proposed check is effectively "does the figure I initially got still use the same canvas as initially?" Also none of these attributes should ever be None, AFAICT.

Alternatively, we could additionally recordself._figure = ax.figure in the constructor and checkself._figure.canvas is self.canvas; again this should sidestep the None problems?

Copy link

MemberAuthor

tacaswell commentedMar 31, 2023

Alternatively, we could additionally record self._figure = ax.figure in the constructor and check self._figure.canvas is self.canvas; again this should sidestep the None problems?

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 toself.canvas is self.canvas.figure.canvas

tacaswell force-pushed thefix_removed_widget_axes branch from54db976 to7100b57Compare

March 31, 2023 20:08

anntzer reviewed

Mar 31, 2023

View reviewed changes

lib/matplotlib/widgets.py OutdatedShow resolvedHide resolved

anntzer reviewed

Mar 31, 2023

View reviewed changes

Copy link

Contributor

anntzer left a comment

Choose a reason for hiding this comment

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

Modulo CI, and with an optional additional comment.

anntzer approved these changes

Mar 31, 2023

View reviewed changes

Copy link

MemberAuthor

tacaswell commentedApr 3, 2023

Hmm, we are back to not being careful enough...but only an azure?!

Copy link

Contributor

anntzer commentedApr 3, 2023

That's a bit fishy, but perhaps just change all theself.ax.draw_artist(artist) byartist.draw(self.canvas.get_renderer())? (which should bypass the problem of whetherself.ax.figure is None or not -- anyways, having draw_artist be an Axes method always seemed a bit weird to me (I do realize that historically we cached the renderer on the axes, but that's not really the logical place to have it)).

jklymak added the status: needs revision label

Jun 13, 2023

QuLogic modified the milestones:v3.7.2,v3.7.3

Jul 5, 2023

QuLogic modified the milestones:v3.7.3,v3.8.0

Sep 9, 2023

ksunden removed this from thev3.8.0 milestone

Sep 12, 2023

github-actionsbot added the status: needs rebase label

Jul 4, 2024

tacaswell force-pushed thefix_removed_widget_axes branch froma9f4c6c todb2176fCompare

July 18, 2025 00:15

github-actionsbot added topic: widgets/UI and removed status: needs rebase labels

Jul 18, 2025

tacaswell requested review fromanntzer andtimhoffm

July 18, 2025 00:15

tacaswell added this to thev3.11.0 milestone

Jul 18, 2025

tacaswell removed the status: needs revision label

Jul 18, 2025

tacaswell mentioned this pull request

Jul 18, 2025

[Bug]: fig.savefig throws error after radiobutton axes is removed#30325

Closed

anntzer reviewed

Jul 18, 2025

View reviewed changes

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():

Copy link

Contributor

anntzerJul 18, 2025

Choose a reason for hiding this comment

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

Should the canvas is not None check rather go to a newAxesWidget.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.

Copy link

Member

timhoffmJul 18, 2025

Choose a reason for hiding this comment

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

I agree. This belongs logically in anAxesWidget.ignore().

anntzer approved these changes

Jul 18, 2025

View reviewed changes

Copy link

Contributor

anntzer left a comment

Choose a reason for hiding this comment

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

I think the patch could be better, but this is also correct.

Copy link

MemberAuthor

tacaswell commentedJul 18, 2025•
edited
Loading

I tried putting the check inWidget.ignore which breaks for other things which is why I originally went with the checks in_clear.

Added a new method and a couple ofsuper() calls.

timhoffm approved these changes

Jul 18, 2025

View reviewed changes

lib/matplotlib/tests/test_widgets.py Outdated

Comment on lines 1762 to 1782

		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)

Copy link

Member

timhoffmJul 18, 2025

Choose a reason for hiding this comment

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

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?