Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
MNT: when clearing an Axes via clear/cla fully detach children#24627
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Reset the Axes and Figure of the children to None to help break cycles.Closesmatplotlib#6982
c68d4de toffcc8d3Compare| self._children = [] | ||
| for chld in old_children: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there a reason for not doingfor chld in getattr(self, "_children", []): ... before resetting _children?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I am more worried about the state where the Axes thinks the Artist is in the draw list rather than the case where the Artist still thinks it is in the Axes (which has been the status quo).
My first attempt was
old_children, self._children = self._children, []but that had issues where either you checkself._children before it exists or you get other attribute errors because other parts of the code are using the existence ofself._children to determine if we are still in the init method or not.
I also thought about doingfor chld in self._chidren: chld.remove() but was trying to avoid a lot of thrashing / maybe some premature optimization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
not sure if compressing to one line is better or worse ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Perhaps a good idea to first get rid of this "not having _children" problem by pre-initing it early in__init__? e.g. something like
diff --git i/lib/matplotlib/axes/_base.py w/lib/matplotlib/axes/_base.pyindex 3e665cda42..9eb65d3fc4 100644--- i/lib/matplotlib/axes/_base.py+++ w/lib/matplotlib/axes/_base.py@@ -665,6 +665,8 @@ class _AxesBase(martist.Artist): self.set_box_aspect(box_aspect) self._axes_locator = None # Optionally set via update(kwargs).+ self._children = []+ # placeholder for any colorbars added that use this Axes. # (see colorbar.py): self._colorbars = []@@ -1078,7 +1080,7 @@ class _AxesBase(martist.Artist): self.transScale.set( mtransforms.blended_transform_factory( self.xaxis.get_transform(), self.yaxis.get_transform()))- for line in getattr(self, "_children", []): # Not set during init.+ for line in self._children: if not isinstance(line, mlines.Line2D): continue try:@@ -2936,22 +2938,15 @@ class _AxesBase(martist.Artist): x_stickies = y_stickies = np.array([]) if self.use_sticky_edges:- # Only iterate over Axes and artists if needed. The check for- # ``hasattr(ax, "_children")`` is necessary because this can be- # called very early in the Axes init process (e.g., for twin Axes)- # when these attributes don't even exist yet, in which case- # `get_children` would raise an AttributeError. if self._xmargin and scalex and self.get_autoscalex_on(): x_stickies = np.sort(np.concatenate([ artist.sticky_edges.x for ax in self._shared_axes["x"].get_siblings(self)- if hasattr(ax, "_children") for artist in ax.get_children()])) if self._ymargin and scaley and self.get_autoscaley_on(): y_stickies = np.sort(np.concatenate([ artist.sticky_edges.y for ax in self._shared_axes["y"].get_siblings(self)- if hasattr(ax, "_children") for artist in ax.get_children()])) if self.get_xscale() == 'log': x_stickies = x_stickies[x_stickies > 0]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That will lead toAttributeErrors related to the Axes title attributes (and that was the point where I stopped debugging and usedgetattr) 🤣
I think the choice is either acceptgetattr or start pulling this string and see what we have to unravel which might spin out to "re-do all of the init logic in the Axes family".
Does this need some refactor now after#24634? |
522cc01 to146b478Comparedropped the |
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Reset the Axes and Figure of the children to None to help break cycles.
Closes#6982
PR Summary
PR Checklist
Documentation and Tests
pytestpasses)I do not think this needs any additional documentation.