Fixed 1656: Animation is Garbage Collected if not explicitly stored #8214
Conversation
lib/matplotlib/animation.py
Outdated
| @@ -874,6 +874,12 @@ def __init__(self, fig, event_source=None, blit=False): | |||
| if self._blit: | |||
| self._setup_blit() | |||
|
|
|||
| # Saves the animation is the figure class to prevent Python's garbage | |||
| # collection from deleteing the animation. | |||
| if (not hasattr(fig, 'animations')): | |||
There was a problem hiding this comment.
If we are going to do this, add it in the __init__ instead of using a hasattr check.
There was a problem hiding this comment.
I have added the animations attribute in figure init instead of the hasattr.
| try: | ||
| length = len(fig.animations) | ||
| assert(length != 0) | ||
| except AttributeError: |
There was a problem hiding this comment.
I don't think the explicit try..except adds much here, just the bare assert is good enough in both places.
|
@raychut Thank you for your work on this! I am concerned about this for exactly the reasons that @dopplershift listed in the original issue (creates circular coupling). If we do this for animation, we should also do this for widgets. Maybe instead of |
|
I would just stick everything in a private |
|
@tacaswell @anntzer I personally agree in changing the name into a private function and/or change to a more generic variable name to store other |
|
So I like the suggestion about fig = plt.figure()
anim = matplotlib.animation.ArtistAnimation(...)
del anim # Animation resources not released due to reference in figureSo we've traded one confusing behavior where resources are being freed when not expected with another where resources aren't freed. I'm not sure what to do... |
|
@dopplershift I've changed the attribute name from That is a tricky point and I guess it is the consequences from not changing the API. I'm not sure what to do either....however even before this change using |
|
@raychut Python's I'm curious if any of the other matplotlib devs would like to weigh in on our conundrum here. |
|
@dopplershift's argument in #1656 ("I still don't consider this a problem, since FuncAnimation and co. are classes; nowhere in Python do we create an instance of a class and expect it to go on living if we don't assign a name to it.") looks reasonable to me, so count me as -1 on this change (i.e. let's leave things as they are)... as of now. |
|
I don't know if this point has been raised, but... *technically* an
animation doesn't have to be limited to a single figure. This was a design
choice early on in order to have multiple windows (or more specifically,
canvas objects) run a synchronized animation in an embedded application.
Since then, we gained the ability to share the timer across multiple
animation objects, and it is more pythonic to have modular animations (as I
demonstrate in chapter 3 of my book).
While it probably shouldn't be a part of this PR, perhaps we should
consider figuring out a way to enforce that all artists controlled by an
animation object be limited to a single figure?
…On Wed, Apr 12, 2017 at 1:53 AM, Antony Lee ***@***.***> wrote:
@dopplershift <https://github.com/dopplershift>'s argument in #1656
<#1656> ("I still don't
consider this a problem, since FuncAnimation and co. are classes; nowhere
in Python do we create an instance of a class and expect it to go on living
if we don't assign a name to it.") looks reasonable to me, so count me as
-1 on this change (i.e. let's leave things as they are)... as of now.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#8214 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-Gv_UDEspZokTay1HqHwcTn9N-faks5rvGbbgaJpZM4MVC_m>
.
|
|
I am still torn on this. On one hand, it does 'fix' a common mistake, on the other hand it creates one more circular reference, one more implicit bit of 'magic', and it feels like we are working against the language. Directly calling This issue is now documented clearly, maybe it also needs a call-out box? http://matplotlib.org/api/animation_api.html?highlight=animation#animation |
why? |
|
Closing as stale, and not quite reaching consensus... Thanks for the contribution! |
Fixed #1656 where the animation object somehow gets deleted when not assigned a variable to it. Had animation.py store it as an attribute of the figure as suggested so that user behavior doesn't change