FuncAnimation with generator defaults to arbitrary save_count=100

[mforbes]

Bug report

Bug summary

When using matplotlib.animation.FuncAnimation with a generator and without specifying save_count, a default value of save_count=100 is used. Instead, the generator should be run until it raises a StopIteration exception.

Code for reproduction

import numpy as np
from matplotlib.animation import FuncAnimation

def get_frames():
    t = 0.0
    x = np.linspace(0, 2*np.pi, 100)
    l, = plt.plot(x, np.sin(x+t))
    txt = plt.title("")
    yield l,  # Initial frame... not stored
    for nt, t in enumerate(np.linspace(0,2*np.pi, 200)):
        l.set_data(x, np.sin(x+t))
        txt.set_text("frame: {}".format(nt))
        yield l,
        
anim = FuncAnimation(plt.gcf(), lambda artists:artists, frames=get_frames())

# To view, use a notebook for example
from IPython.display import HTML
HTML(anim.to_html5_video())

Actual outcome

Only frames 0 to 99 are drawn even though the generator will produce 200 frames (up to 199).

Expected outcome

An animation that runs until the generator raises StopIteration.

Matplotlib version

It looks like this will affect the latest version on GitHub. I tested this with version 1.5.3 from anaconda.

[dopplershift]

Not a bug. Generators are not guaranteed to ever raise StopIteration, which is one of the reasons save_count exists. Just blindly looking for that could lead to an infinite loop.

I'd be open to a pull request that uses a sentinel value for save_count (-1?) to indicate that the generator will finish and to keep iterating until StopIteration. I've not dug in enough to know the full ramifications of this though.

[anntzer]

I agree with the OP and disagree with @dopplershift. The general pattern in Python when someone feeds you an infinite generator is to just keep advancing it forever and ever, and consider the infinite look a user-level bug. np.fromiter(itertools.count(), int) just loops forever, it does not truncate its output after 200 elements.

[dopplershift]

You can disagree with whether save_count was a good idea, but speaking as the designer, there's no argument: save_count is operating exactly as I intended in this case. Ergo, not a bug.

Since we don't want to break backwards compatibility, how do you feel about allowing a save_count to signal that we should just go until StopIteration?

[anntzer]

At least then the default value of save_count should be switched to 100 (instead of None) and this should be reflected in the docs. Setting save_count to None should be understood as no limit.

[WeatherGod]

Careful. We tend to use None to mean default, and 'none' to mean literally none.

…

On Apr 15, 2017 3:55 PM, "Antony Lee" ***@***.***> wrote: At least then the default value of save_count should be switched to 100 (instead of None) and this should be reflected in the docs. Setting save_count to None should be understood as no limit. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8278 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-ErLHgp0COobD6cY9Eo8x0z0QQUjks5rwSCmgaJpZM4Maxt6> .

[tacaswell]

True, but 'none' isn't super meaningful here, going with -1 meaning 'forever' is a c-ism I am not a huge fan of, and as @anntzer points out 'just keep going' is the expected python behavior for None in this sort of case.

[dopplershift]

I'm fine in principle with None as the sentinel value. We have a complication here where None maps to default behavior, which depends on whether we're given a sequence (known length) vs. iterator (unknown length). It's a little convoluted at the moment, and I'm not sure how to utilize None for "just keep reading" without breaking the existing use of None. Unless we don't think it's important to preserve... @tacaswell ?

[tacaswell]

Un-related change:

• change the internal logic to treat save_count as an upper limit and actually set to min(save_count, len(frames)) assuming it has a length, currently it is completely ignored if len(frames) is a thing. You could argue that it should be the users job to slice the input how ever they want before handing it to us (which is what I would say to argue against adding a step kwarg).

option 1

• change default to our own sentinal, if we get that clip to 100
• let None mean length of frames or un-bounded

The only place this will break things is will really break things is if someone is passing in an un-bounded generator + explicitly passing in save_count=None and relying on us to implicitly clip to 100 (rather than clipping them selves). That seems like it should be rare and is a bad idea in any case.

option 2

• leave default as None
• treat None as un-bounded, but still extract a reasonable length len(frames) is a thing

This is simpler, but will break anyone who has been passing in an un-bounded generator.

option 3

• add a UNBOUNDED sentinel to animation, but I like both 1 and 2 better.

won't break anything, but adds extra complexity to get 'expected pythonic behavior'.

not-an-option

• changing the default to 100 as we sometimes ignore this and would interfere with the unrelated thing at the top

Sorry this is rambly.

[anntzer]

Option 2 will only break code that used to pass an unbounded generator and expected it to be clipped at 100, which is likely broken as well (although an infinite loop may indeed qualify "more broken"). So I'd favor that.

[anntzer]

Behavior deprecated with a warning in #10301.

[v-goncharenko]

Seems like conversation eded with some merges, but documentation still have nothing about this kind of behaviour. It's quite confusing especially considering that even StackOverflow have dedicated topic.

Maybe it's worth to just add some note to docs while complex solutions is being developed.

Thanks

[anntzer]

Looks like the deprecation is still in place, "just" needs to be expired. I guess it's a simple

diff --git i/lib/matplotlib/animation.py w/lib/matplotlib/animation.py
index 54c901628..fec0b79a7 100644
--- i/lib/matplotlib/animation.py
+++ w/lib/matplotlib/animation.py
@@ -1690,24 +1690,8 @@ class FuncAnimation(TimedAnimation):
         else:
             if self.save_count is not None:
                 return itertools.islice(self.new_frame_seq(), self.save_count)
-
             else:
-                frame_seq = self.new_frame_seq()
-
-                def gen():
-                    try:
-                        for _ in range(100):
-                            yield next(frame_seq)
-                    except StopIteration:
-                        pass
-                    else:
-                        cbook.warn_deprecated(
-                            "2.2", message="FuncAnimation.save has truncated "
-                            "your animation to 100 frames.  In the future, no "
-                            "such truncation will occur; please pass "
-                            "'save_count' accordingly.")
-
-                return gen()
+                return self.new_frame_seq()
 
     def _init_draw(self):
         # Initialize the drawing either using the given init_func or by

but didn't look more into it.

[v-goncharenko]

Adduce code to reproduce my concern:

import matplotlib.pyplot as plt
import matplotlib.animation as animation
import numpy as np

def generator(high: int=200):
    for i in range(high):
        print(f'Yielding {i}-th step')
        yield i

figure = plt.figure()

line = plt.plot(np.arange(10), np.linspace(-2, 2, 10), 'og')[0]

def update_line(i: int):
    x = np.arange(10)
    y = np.sin(np.arange(10)) + np.random.randn(10) * i / 100
    line.set_data(x, y)
    return (line, )

anim = animation.FuncAnimation(
    figure,
    update_line,
    generator(),
    blit=True,
)

anim.save('example.mp4')
plt.close()

In this code generator are called only 101 times and no warning or error or something is brought to user. It's quite confusing to me withal documentation says nothing about this behaviour.

[HormyAJP]

I just tripped over this issue. I'm very new to matplotlib so I'm coming with fresh eyes, however my feeling is that the current behaviour is very unintuitive. I was only getting a video recording of the first 100 frames and had no idea why. I thought something was broken or my code was screwed up. In the end I had to debug animation.py in matplotlib to figure out the problem.

I'm +1 for some fix on this. If no one wants to pick it up then I'm happy to

[ForceBru]

Indeed, very counterintuitive. My generator is supposed to output thousands of values, but my self.update function as being called exactly 100 times no matter the number of values in the generator, and I don't even get a warning that not all values from the generator are used:

nframes = 50_000
dots_per_frame = 20
anim = animation.FuncAnimation(
    self.figure, self.update, init_func=self.setup,
    frames=itertools.repeat(range(dots_per_frame), nframes),
    interval=interval, blit=True, repeat_delay=500, cache_frame_data=False
)

I noticed that no matter the value of nframes, the code will execute very quickly, so I tried to track the number of invocations of self.update, and it turned out it's called exactly 100 times every time.

Even worse, nowhere in the docs does it say that save_count defaults to 100. While other options have "Defaults to [something]" as the end of their description, this one doesn't say anything about the default value:

save_count : int, optional
The number of values from frames to cache.

Then at the bottom of the page it says:

__init__(self, fig, func, frames=None, init_func=None, fargs=None, save_count=None, *, cache_frame_data=True, **kwargs)[source]
Initialize self. See help(type(self)) for accurate signature.

So okay, the default value is None: ... fargs=None, save_count=None, *, ..., but this doesn't tell me anything, especially about the 100 iterations limit.

This limit can be found only in the code (!) by clicking [source] here: https://matplotlib.org/_modules/matplotlib/animation.html#FuncAnimation.__init__:

if self.save_count is None:
    # If we're passed in and using the default, set save_count to 100.
    self.save_count = 100  # NOW THAT'S UNEXPECTED!
else:
    # itertools.islice returns an error when passed a numpy int instead
    # of a native python int (http://bugs.python.org/issue30537).
    # As a workaround, convert save_count to a native python int.
    self.save_count = int(self.save_count)

I stumbled upon this issue by googling "matplotlib animation updates 100 times".

matplotlib.__version__ == '3.1.2'

[timhoffm]

#8278 (comment)

Looks like the deprecation is still in place, "just" needs to be expired. I guess it's a simple

Unfortunately it's not that simple.

We initialize save_count explicitly to 100 if we don't have a better idea:

matplotlib/lib/matplotlib/animation.py

Line 1643 in 29fe439

self.save_count = 100

We rely on a finite value of save_count.

matplotlib/lib/matplotlib/animation.py

Line 1721 in 29fe439

self._save_seq = self._save_seq[-self.save_count:]

What was the intention of happening after the deprecation? Should it run as lon as frames yields values (maybe case forever)? Or do we want to require an explicit values for iterators and generators?

[anntzer]

Should it run as lon as frames yields values (maybe case forever)?

My opinion is yes (it's just like calling list on an iterator: we assume that the user is responsible for ensuring that the thing is finite, calling list() on an infinite iterator just hangs).

Looks like the deprecation is still in place, "just" needs to be expired. I guess it's a simple...

Yeah, I'm not sure I put in the deprecation correctly, we likely need another deprecation round if we want to be safe.

[timhoffm]

I also think that we need a second deprecation.

But more importantly, we have to understand how this should be modified if we don't have any save_count:

matplotlib/lib/matplotlib/animation.py

Line 1721 in 29fe439

self._save_seq = self._save_seq[-self.save_count:]

[QuLogic]

I was going to remove this in 3.3, but I see we have some outstanding decisions. The way I see it, we have 3 options for handling _save_seq when save_count is None:

• Don't trim the list; this is the minimal fixup, but wastes memory if the animation is intended to be infinite.
• Don't save anything; this doesn't waste any memory, but means you can't watch, then save a movie.
• Add an option to do either one (somewhere.)

[dopplershift]

I think I lean towards don't trim, which is the most consistent with the original, albeit flawed, design and the current operation.

[QuLogic]

The 100 save_count default was removed in #24131