Deprecate truncating saved unsized anims to 100 frames. #10301
Conversation
c7a4328 to
eaf3e66
Compare
lib/matplotlib/animation.py
Outdated
| for anim, d in zip(all_anim, data): | ||
| if d is _truncated_sentinel: |
There was a problem hiding this comment.
Just for my learning, why do you need _trundated_sentinel in here, versus just counting to 100?
There was a problem hiding this comment.
To only trigger the warning in the case the user passed in a generator. If the user passed in just a very long but explicit list, the previous code would (correctly) not truncate it, and we don't want to trigger a warning in this case.
There was a problem hiding this comment.
Can't you do this in a clearer way (i.e. w/o itertools.chain)? I guess I'd be OK w/ this construct if it saves a bunch of code, or maybe this is really the only way to do it, but it seems this check could be less opaque.
There was a problem hiding this comment.
AFAICT I need to tag a sentinel at the end of the generator. But this needs to be done without actually running the generator first (because that can require arbitrarily large resources); so itertools.chain is literally the way to do that.
Alternatively I would need to keep another flag around saying "this is a generator-based animation, please check whether we reached the 100th frame", which I guess works too but I am not sure it's simpler...
There was a problem hiding this comment.
I think this is a reasonable way to implement this, it keeps the state all in one place.
There was a problem hiding this comment.
If this is the best way to do it, it would help if there was a bit of a comment in the code explaining what is going on, in case you both decide to take up extreme crocheting and don't stop around here anymore.
There was a problem hiding this comment.
Actually I think I figured out a less obfuscated way to implement this... pushed.
eaf3e66 to
acb6d5c
Compare
lib/matplotlib/animation.py
Outdated
| "2.2", "FuncAnimation.save has truncated your animation " | ||
| "to 100 frames. In the future, no such truncation will " | ||
| "occur; please pass 'save_count' accordingly.") | ||
| try: |
There was a problem hiding this comment.
Sorry to be a pain, but why is this second block needed? Are you going to keep returning frames even though the saving is not saving any more?
There was a problem hiding this comment.
because I'm an idiot...
To test that the deprecation is working, uncomment the `ani.save(...)` line in the simple_anim.py example and modify the anim's save_count. Animations that happened to generate exactly 100 frames without truncation will incorrectly trigger a warning; not sure we can do much about it (because after the 100th frame is generated there is no way to know whether another frame is coming except by actually calling next() on the generator, which we do not want to do).
acb6d5c to
15c6d59
Compare
To test that the deprecation is working, uncomment the
ani.save(...)line in the simple_anim.py example and modify the anim's save_count.
Animations that happened to generate exactly 100 frames without
truncation will incorrectly trigger a warning; not sure we can do much
about it (because after the 100th frame is generated there is no way to
know whether another frame is coming except by actually calling next()
on the generator, which we do not want to do).
cf discussion in #8278.
PR Summary
PR Checklist