-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[ENH]: should Axes hold references to widgets? #24373
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
Comments
See also #16221 and linked issues. |
Thanks for the reminder on #16221. At least for the case of widgets, I feel that holding explicit references is simpler than the lambda construct (explicit is better than implicit). |
To be clear, I don't have a strong preference implementation-wise, it was just to point out an earlier discussion on the same topic. |
@anntzer in #16221 you complain about the use of weak references in the callback registry. Proposals to address the current issue include your lambda trick and tracking strong references in the Figure or Axes. Do you also have in mind a solution based on switching from weak to strong references in the callback registry? Or would that have been your preference from the start, but it is now too late to change it? |
I would certainly prefer using strong references (the lambda trick is basically just a way to get strong references for cheap). I think it's possible to even have a proper deprecation period when changing that (basically emit a warning if a method proxy gets gc'ed from the callbackregistry if it didn't get disconnected before -- untested, though). |
See also #26881 ("Provide a standard place to manually register widgets/animations on a figure/artist/etc.") for an alternative suggestion. |
Problem
People often forget to hold a reference to a widget so that it‘s getting garbage collected, c.f. #24372.
Proposed solution
Track widgets in a (maybe private) Axes attribute, as we do for artists.
The widget‘s artists (=visual part of the widget) are referenced, so there is a tight relationship. We should add the widget objects themselves (=logic part of the widget) as well.
The text was updated successfully, but these errors were encountered: