Merge pull request #24131 from oscargus/animationdeprecations

greglucas · web-flow · commit c9fc6a37668f · 2022-12-29T14:21:15.000-07:00
Deprecate attributes and expire deprecation in animation
diff --git a/doc/api/next_api_changes/behavior/24131-OG.rst b/doc/api/next_api_changes/behavior/24131-OG.rst
@@ -0,0 +1,6 @@
+``FuncAnimation(save_count=None)``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Passing ``save_count=None`` to `.FuncAnimation` no longer limits the number
+of frames to 100. Make sure that it either can be inferred from *frames*
+or provide an integer *save_count*.
diff --git a/doc/api/next_api_changes/deprecations/24131-OG.rst b/doc/api/next_api_changes/deprecations/24131-OG.rst
@@ -0,0 +1,5 @@
+``Animation`` attributes
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+The attributes ``repeat`` of `.TimedAnimation` and subclasses and
+``save_count`` of `.FuncAnimation` are considered private and deprecated.
diff --git a/doc/missing-references.json b/doc/missing-references.json
diff --git a/examples/animation/animate_decay.py b/examples/animation/animate_decay.py
@@ -50,5 +50,7 @@ def run(data):
 
     return line,
 
-ani = animation.FuncAnimation(fig, run, data_gen, interval=100, init_func=init)
+# Only save last 100 frames, but run forever
+ani = animation.FuncAnimation(fig, run, data_gen, interval=100, init_func=init,
+                              save_count=100)
 plt.show()
diff --git a/examples/animation/rain.py b/examples/animation/rain.py
@@ -67,5 +67,5 @@ def update(frame_number):
 
 
 # Construct the animation, using the update function as the animation director.
-animation = FuncAnimation(fig, update, interval=10)
+animation = FuncAnimation(fig, update, interval=10, save_count=100)
 plt.show()
diff --git a/examples/animation/strip_chart.py b/examples/animation/strip_chart.py
@@ -61,6 +61,6 @@ def emitter(p=0.1):
 
 # pass a generator in "emitter" to produce data for the update func
 ani = animation.FuncAnimation(fig, scope.update, emitter, interval=50,
-                              blit=True)
+                              blit=True, save_count=100)
 
 plt.show()
diff --git a/examples/animation/unchained.py b/examples/animation/unchained.py
@@ -69,5 +69,5 @@ def update(*args):
     return lines
 
 # Construct the animation, using the update function as the animation director.
-anim = animation.FuncAnimation(fig, update, interval=10)
+anim = animation.FuncAnimation(fig, update, interval=10, save_count=100)
 plt.show()
diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py
@@ -247,7 +247,7 @@ class MovieWriter(AbstractMovieWriter):
         The format used in writing frame data, defaults to 'rgba'.
     fig : `~matplotlib.figure.Figure`
         The figure to capture data from.
-        This must be provided by the sub-classes.
+        This must be provided by the subclasses.
     """
 
     # Builtin writer subclasses additionally define the _exec_key and _args_key
@@ -1083,7 +1083,7 @@ def _pre_composite_to_white(color):
             frame_number = 0
             # TODO: Currently only FuncAnimation has a save_count
             #       attribute. Can we generalize this to all Animations?
-            save_count_list = [getattr(a, 'save_count', None)
+            save_count_list = [getattr(a, '_save_count', None)
                                for a in all_anim]
             if None in save_count_list:
                 total_frames = None
@@ -1236,7 +1236,7 @@ def to_html5_video(self, embed_limit=None):
         This saves the animation as an h264 video, encoded in base64
         directly into the HTML5 video tag. This respects :rc:`animation.writer`
         and :rc:`animation.bitrate`. This also makes use of the
-        ``interval`` to control the speed, and uses the ``repeat``
+        *interval* to control the speed, and uses the *repeat*
         parameter to decide whether to loop.
 
         Parameters
@@ -1299,7 +1299,7 @@ def to_html5_video(self, embed_limit=None):
             options = ['controls', 'autoplay']
 
             # If we're set to repeat, make it loop
-            if hasattr(self, 'repeat') and self.repeat:
+            if getattr(self, '_repeat', False):
                 options.append('loop')
 
             return VIDEO_TAG.format(video=self._base64_video,
@@ -1320,17 +1320,18 @@ def to_jshtml(self, fps=None, embed_frames=True, default_mode=None):
         embed_frames : bool, optional
         default_mode : str, optional
             What to do when the animation ends. Must be one of ``{'loop',
-            'once', 'reflect'}``. Defaults to ``'loop'`` if ``self.repeat``
-            is True, otherwise ``'once'``.
+            'once', 'reflect'}``. Defaults to ``'loop'`` if the *repeat*
+            parameter is True, otherwise ``'once'``.
         """
         if fps is None and hasattr(self, '_interval'):
             # Convert interval in ms to frames per second
             fps = 1000 / self._interval
 
         # If we're not given a default mode, choose one base on the value of
-        # the repeat attribute
+        # the _repeat attribute
         if default_mode is None:
-            default_mode = 'loop' if self.repeat else 'once'
+            default_mode = 'loop' if getattr(self, '_repeat',
+                                             False) else 'once'
 
         if not hasattr(self, "_html_representation"):
             # Can't open a NamedTemporaryFile twice on Windows, so use a
@@ -1394,13 +1395,12 @@ class TimedAnimation(Animation):
     blit : bool, default: False
         Whether blitting is used to optimize drawing.
     """
-
     def __init__(self, fig, interval=200, repeat_delay=0, repeat=True,
                  event_source=None, *args, **kwargs):
         self._interval = interval
         # Undocumented support for repeat_delay = None as backcompat.
         self._repeat_delay = repeat_delay if repeat_delay is not None else 0
-        self.repeat = repeat
+        self._repeat = repeat
         # If we're not given an event source, create a new timer. This permits
         # sharing timers between animation objects for syncing animations.
         if event_source is None:
@@ -1417,7 +1417,7 @@ def _step(self, *args):
         # back.
         still_going = super()._step(*args)
         if not still_going:
-            if self.repeat:
+            if self._repeat:
                 # Restart the draw loop
                 self._init_draw()
                 self.frame_seq = self.new_frame_seq()
@@ -1437,6 +1437,8 @@ def _step(self, *args):
         self.event_source.interval = self._interval
         return True
 
+    repeat = _api.deprecate_privatize_attribute("3.7")
+
 
 class ArtistAnimation(TimedAnimation):
     """
@@ -1593,7 +1595,7 @@ def init_func() -> iterable_of_artists
         Additional arguments to pass to each call to *func*. Note: the use of
         `functools.partial` is preferred over *fargs*. See *func* for details.
 
-    save_count : int, default: 100
+    save_count : int, optional
         Fallback for the number of values from *frames* to cache. This is
         only used if the number of frames cannot be inferred from *frames*,
         i.e. when it's an iterator without length or a generator.
@@ -1618,7 +1620,6 @@ def init_func() -> iterable_of_artists
         Whether frame data is cached.  Disabling cache might be helpful when
         frames contain large objects.
     """
-
     def __init__(self, fig, func, frames=None, init_func=None, fargs=None,
                  save_count=None, *, cache_frame_data=True, **kwargs):
         if fargs:
@@ -1631,7 +1632,7 @@ def __init__(self, fig, func, frames=None, init_func=None, fargs=None,
         # Amount of framedata to keep around for saving movies. This is only
         # used if we don't know how many frames there will be: in the case
         # of no generator or in the case of a callable.
-        self.save_count = save_count
+        self._save_count = save_count
         # Set up a function that creates a new iterable when needed. If nothing
         # is passed in for frames, just use itertools.count, which will just
         # keep counting from 0. A callable passed in for frames is assumed to
@@ -1651,19 +1652,31 @@ def iter_frames(frames=frames):
             else:
                 self._iter_gen = lambda: iter(frames)
             if hasattr(frames, '__len__'):
-                self.save_count = len(frames)
+                self._save_count = len(frames)
+                if save_count is not None:
+                    _api.warn_external(
+                        f"You passed in an explicit {save_count=} "
+                        "which is being ignored in favor of "
+                        f"{len(frames)=}."
+                    )
         else:
             self._iter_gen = lambda: iter(range(frames))
-            self.save_count = frames
-
-        if self.save_count is None:
-            # If we're passed in and using the default, set save_count to 100.
-            self.save_count = 100
-        else:
-            # itertools.islice returns an error when passed a numpy int instead
-            # of a native python int (https://bugs.python.org/issue30537).
-            # As a workaround, convert save_count to a native python int.
-            self.save_count = int(self.save_count)
+            self._save_count = frames
+            if save_count is not None:
+                _api.warn_external(
+                    f"You passed in an explicit {save_count=} which is being "
+                    f"ignored in favor of {frames=}."
+                )
+        if self._save_count is None and cache_frame_data:
+            _api.warn_external(
+                f"{frames=!r} which we can infer the length of, "
+                "did not pass an explicit *save_count* "
+                f"and passed {cache_frame_data=}.  To avoid a possibly "
+                "unbounded cache, frame data caching has been disabled. "
+                "To suppress this warning either pass "
+                "`cache_frame_data=False` or `save_count=MAX_FRAMES`."
+            )
+            cache_frame_data = False
 
         self._cache_frame_data = cache_frame_data
 
@@ -1690,26 +1703,18 @@ def new_saved_frame_seq(self):
             self._old_saved_seq = list(self._save_seq)
             return iter(self._old_saved_seq)
         else:
-            if self.save_count is not None:
-                return itertools.islice(self.new_frame_seq(), self.save_count)
-
-            else:
+            if self._save_count is None:
                 frame_seq = self.new_frame_seq()
 
                 def gen():
                     try:
-                        for _ in range(100):
+                        while True:
                             yield next(frame_seq)
                     except StopIteration:
                         pass
-                    else:
-                        _api.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()
+            else:
+                return itertools.islice(self.new_frame_seq(), self._save_count)
 
     def _init_draw(self):
         super()._init_draw()
@@ -1747,10 +1752,7 @@ def _draw_frame(self, framedata):
         if self._cache_frame_data:
             # Save the data for potential saving of movies.
             self._save_seq.append(framedata)
-
-        # Make sure to respect save_count (keep only the last save_count
-        # around)
-        self._save_seq = self._save_seq[-self.save_count:]
+            self._save_seq = self._save_seq[-self._save_count:]
 
         # Call the func with framedata and args. If blitting is desired,
         # func needs to return a sequence of any artists that were modified.
@@ -1776,3 +1778,5 @@ def _draw_frame(self, framedata):
 
             for a in self._drawn_artists:
                 a.set_animated(self._blit)
+
+    save_count = _api.deprecate_privatize_attribute("3.7")
diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py
@@ -1,6 +1,7 @@
 import os
 from pathlib import Path
 import platform
+import re
 import subprocess
 import sys
 import weakref
@@ -87,7 +88,7 @@ def test_null_movie_writer(anim):
     # output to an opaque background
     for k, v in savefig_kwargs.items():
         assert writer.savefig_kwargs[k] == v
-    assert writer._count == anim.save_count
+    assert writer._count == anim._save_count
 
 
 @pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim'])
@@ -233,8 +234,11 @@ def test_animation_repr_html(writer, html, want, anim):
         assert want in html
 
 
-@pytest.mark.parametrize('anim', [dict(frames=iter(range(5)))],
-                         indirect=['anim'])
+@pytest.mark.parametrize(
+    'anim',
+    [{'save_count': 10, 'frames': iter(range(5))}],
+    indirect=['anim']
+)
 def test_no_length_frames(anim):
     anim.save('unused.null', writer=NullMovieWriter())
 
@@ -330,9 +334,11 @@ def frames_generator():
 
             yield frame
 
+    MAX_FRAMES = 100
     anim = animation.FuncAnimation(fig, animate, init_func=init,
                                    frames=frames_generator,
-                                   cache_frame_data=cache_frame_data)
+                                   cache_frame_data=cache_frame_data,
+                                   save_count=MAX_FRAMES)
 
     writer = NullMovieWriter()
     anim.save('unused.null', writer=writer)
@@ -372,7 +378,9 @@ def animate(i):
             return return_value
 
     with pytest.raises(RuntimeError):
-        animation.FuncAnimation(fig, animate, blit=True)
+        animation.FuncAnimation(
+            fig, animate, blit=True, cache_frame_data=False
+        )
 
 
 def test_exhausted_animation(tmpdir):
@@ -440,3 +448,61 @@ def animate(i):
 
     # 5th frame's data
     ax.plot(x, np.sin(x + 4 / 100))
+
+
+@pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim'])
+def test_save_count_override_warnings_has_length(anim):
+
+    save_count = 5
+    frames = list(range(2))
+    match_target = (
+        f'You passed in an explicit {save_count=} '
+        "which is being ignored in favor of "
+        f"{len(frames)=}."
+    )
+
+    with pytest.warns(UserWarning, match=re.escape(match_target)):
+        anim = animation.FuncAnimation(
+            **{**anim, 'frames': frames, 'save_count': save_count}
+        )
+    assert anim._save_count == len(frames)
+    anim._init_draw()
+
+
+@pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim'])
+def test_save_count_override_warnings_scaler(anim):
+    save_count = 5
+    frames = 7
+    match_target = (
+        f'You passed in an explicit {save_count=} ' +
+        "which is being ignored in favor of " +
+        f"{frames=}."
+    )
+
+    with pytest.warns(UserWarning, match=re.escape(match_target)):
+        anim = animation.FuncAnimation(
+            **{**anim, 'frames': frames, 'save_count': save_count}
+        )
+
+    assert anim._save_count == frames
+    anim._init_draw()
+
+
+@pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim'])
+def test_disable_cache_warning(anim):
+    cache_frame_data = True
+    frames = iter(range(5))
+    match_target = (
+        f"{frames=!r} which we can infer the length of, "
+        "did not pass an explicit *save_count* "
+        f"and passed {cache_frame_data=}.  To avoid a possibly "
+        "unbounded cache, frame data caching has been disabled. "
+        "To suppress this warning either pass "
+        "`cache_frame_data=False` or `save_count=MAX_FRAMES`."
+    )
+    with pytest.warns(UserWarning, match=re.escape(match_target)):
+        anim = animation.FuncAnimation(
+            **{**anim, 'cache_frame_data': cache_frame_data, 'frames': frames}
+        )
+    assert anim._cache_frame_data is False
+    anim._init_draw()
