-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
add cache_frame_data
kwarg into FuncAnimation
. fixes #8528.
#11894
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
add cache_frame_data
kwarg into FuncAnimation
. fixes #8528.
#11894
Conversation
05bd0c3
to
6254645
Compare
But Is this solving a problem you've run into? The discussion in #8528 indicates that removing this did not solve the issue for the user there. |
Thanks for your feedback, @dopplershift My understanding is that Below is a code snippet which exhibits a memory leak at about 1MB/sec before this fix.
Another way to interpret this bug is that Why does |
If you call save on an animation that was consuming from a generator, add |
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.
As implemented this is removing the ability to save an animation feed from a generator with the same data you just saw.
Currently the last save_count
frames will be saved. This implementation will save the frist save_count
of re-calling the generator function. If you passed in a generator object, and let it be exhausted I think this implementation will save no frames.
I do like the test, but it should check that all but save_count
have been gc'd.
If you change your generator to run through 10k iterations and change We probably should change the logic on There probably should be a flag to disable caching. It is the right thing in many but not all cases. |
Thanks for your feedkback, @tacaswell So, if I understood your comment correctly, Animation is required to support saving its content even after it has already been drawn/shown interactively. Assuming it's required to support save() even after drawing is complete, I'm afraid to say that this contract forces the implementation of Animation expensive because it needs to keep all frame data. Having said that, I like your last comment on having a flag to disable caching in FuncAnimation. |
This is the current behavior (and has been there from when the code came in in 2010 70e401f) so we can not change it with out a deprecation cycle. This is one of the challenges of working on these old, big projects, it is easy to pick any part of the API and say "It would be better if...." but someone is the past thought the current API was a good one and it is likely that there are users depending on that behavior that we can not break without warning (and a good reason). https://xkcd.com/1172/ is real!
This only gets expensive under some conditions (consuming from a generator that generates and yields objects with a memory footpirnt). I have personally used this feature when working interactively (make Maybe Could this problem also be solved by passing |
6254645
to
017d88b
Compare
cache_frame_data
kwarg into FuncAnimation
. fixes #8528.
Thanks for your detailed/helpful feedback, @tacaswell |
48c3d9a
to
ad7103a
Compare
ad7103a
to
f8b14c8
Compare
Coverage is complaining about https://codecov.io/gh/matplotlib/matplotlib/compare/e7719da6cad1d34c7a199833a822f4d2ec076ac8...f8b14c8fc057ef4ed0a67c85df007f5062a4879d/src/lib/matplotlib/tests/test_animation.py?before=lib/matplotlib/tests/test_animation.py#L289 which is a bit odd. Otherwise 👍 |
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.
Modulo a whats_new
entry in https://github.com/matplotlib/matplotlib/tree/master/doc/users/next_whats_new and maybe that extra assertion in the test.
Thanks for following up on this @pclove1 ! |
@tacaswell Thanks for your helpful reviews. |
3d4f958
to
1f908e6
Compare
Squashed commits and rebased against the latest master head |
Travis failure is #13137 and unrelated to the PR. |
Thanks, and congratulations on your first contribution to matplotlib. Hope to see you back some time. |
Like wise. |
PR Summary
cache_frame_data
kwarg intoFuncAnimation
.PR Checklist