-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
FIX: Handle AxesWidget cleanup after failed init #30935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d65638e to
5bc69c9
Compare
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
|
Maybe instead of defining diff --git i/lib/matplotlib/widgets.py w/lib/matplotlib/widgets.py
index 5e87abe08d..9418794dcf 100644
--- i/lib/matplotlib/widgets.py
+++ w/lib/matplotlib/widgets.py
@@ -120,16 +120,6 @@ class AxesWidget(Widget):
self._cids = []
self._blit_background_id = None
- def __del__(self):
- blit_background_id = getattr(self, '_blit_background_id', None)
- # __del__ may be called on a partially initialized object, e.g.,
- # when __init__ raises. Therefore, we handle missing attributes
- # gracefully.
- if blit_background_id is not None:
- canvas = getattr(self, 'canvas', None)
- if canvas is not None:
- canvas._release_blit_background_id(blit_background_id)
-
canvas = property(
lambda self: getattr(self.ax.get_figure(root=True), 'canvas', None)
)
@@ -170,7 +160,10 @@ class AxesWidget(Widget):
good enough for all existing widgets.
"""
if self._blit_background_id is None:
- self._blit_background_id = self.canvas._get_blit_background_id()
+ bbid = self.canvas._get_blit_background_id()
+ import weakref
+ weakref.finalize(self, self.canvas._release_blit_background_id, bbid)
+ self._blit_background_id = bbid
self.canvas._blit_backgrounds[self._blit_background_id] = background
def _load_blit_background(self):? |
|
If my patch works as is (well, moving the import to where it should be) I'd rather have that in directly, otherwise it can be punted. |
|
@anntzer OK, thanks. I've updated this PR with your suggested fix. The test passes. |
668304d to
eeb61a5
Compare
eeb61a5 to
b60db73
Compare
|
Looks like the pyi file needs to be updated for the removal of |
|
@anntzer Fixed, thanks! |
* FIX: Handle AxesWidget cleanup after failed init * Apply code review suggestions * Update lib/matplotlib/tests/test_widgets.py Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> * Remove __del__ method * Remove AxesWidget.__del__ from widgets.pyi --------- Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
* FIX: Handle AxesWidget cleanup after failed init * Apply code review suggestions * Update lib/matplotlib/tests/test_widgets.py Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> * Remove __del__ method * Remove AxesWidget.__del__ from widgets.pyi --------- Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
* FIX: Handle AxesWidget cleanup after failed init * Apply code review suggestions * Update lib/matplotlib/tests/test_widgets.py Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> * Remove __del__ method * Remove AxesWidget.__del__ from widgets.pyi --------- Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
PR summary
In 3.11.0.dev,
AxesWidget.__del__is called on partially constructed selectors when__init__raises early (e.g.,EllipseSelector(ax, foo="bar")with an invalid kwarg). Because__init__never runs, attributes like_blit_background_idandcanvasare not set, so__del__givesException ignored while calling deallocator <function AxesWidget.__del__ at 0x108598b40>:and raises anAttributeErrorduring garbage collection. This surfaces asPytestUnraisableExceptionWarningin tests in downstream packages.See, e.g., https://github.com/astropy/regions/actions/runs/20730257507/job/59516157314
This bug was introduced in #30591.
Minimal example (also added as a regression test):
PR checklist