Merge pull request #16078 from anntzer/animstart

timhoffm · web-flow · commit eb18cb24ce87 · 2020-01-05T12:19:37.000+01:00
Refactor a bit animation start/save interaction.
diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py
@@ -939,9 +939,11 @@ def _start(self, *args):
         Starts interactive animation. Adds the draw frame command to the GUI
         handler, calls show to start the event loop.
         """
+        # Do not start the event source if saving() it.
+        if self._fig.canvas.is_saving():
+            return
         # First disconnect our draw event handler
         self._fig.canvas.mpl_disconnect(self._first_draw_id)
-        self._first_draw_id = None  # So we can check on save
 
         # Now do any initial draw
         self._init_draw()
@@ -1057,14 +1059,6 @@ def func(current_frame: int, total_frames: int) -> Any
         if savefig_kwargs is None:
             savefig_kwargs = {}
 
-        # Need to disconnect the first draw callback, since we'll be doing
-        # draws. Otherwise, we'll end up starting the animation.
-        if self._first_draw_id is not None:
-            self._fig.canvas.mpl_disconnect(self._first_draw_id)
-            reconnect_first_draw = True
-        else:
-            reconnect_first_draw = False
-
         if fps is None and hasattr(self, '_interval'):
             # Convert interval in ms to frames per second
             fps = 1000. / self._interval
@@ -1123,8 +1117,11 @@ def func(current_frame: int, total_frames: int) -> Any
             _log.info("Disabling savefig.bbox = 'tight', as it may cause "
                       "frame size to vary, which is inappropriate for "
                       "animation.")
+        # canvas._is_saving = True makes the draw_event animation-starting
+        # callback a no-op.
         with mpl.rc_context({'savefig.bbox': None}), \
-             writer.saving(self._fig, filename, dpi):
+             writer.saving(self._fig, filename, dpi), \
+             cbook._setattr_cm(self._fig.canvas, _is_saving=True):
             for anim in all_anim:
                 anim._init_draw()  # Clear the initial frame
             frame_number = 0
@@ -1145,11 +1142,6 @@ def func(current_frame: int, total_frames: int) -> Any
                         frame_number += 1
                 writer.grab_frame(**savefig_kwargs)
 
-        # Reconnect signal for first draw if necessary
-        if reconnect_first_draw:
-            self._first_draw_id = self._fig.canvas.mpl_connect('draw_event',
-                                                               self._start)
-
     def _step(self, *args):
         """
         Handler for getting events. By default, gets the next frame in the
diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py
@@ -237,13 +237,8 @@ def test_failing_ffmpeg(tmpdir, monkeypatch):
             make_animation().save("test.mpeg")
 
 
-@pytest.mark.parametrize("cache_frame_data, weakref_assertion_fn", [
-    pytest.param(
-        False, lambda ref: ref is None, id='cache_frame_data_is_disabled'),
-    pytest.param(
-        True, lambda ref: ref is not None, id='cache_frame_data_is_enabled'),
-])
-def test_funcanimation_holding_frames(cache_frame_data, weakref_assertion_fn):
+@pytest.mark.parametrize("cache_frame_data", [False, True])
+def test_funcanimation_cache_frame_data(cache_frame_data):
     fig, ax = plt.subplots()
     line, = ax.plot([], [])
 
@@ -282,4 +277,6 @@ def frames_generator():
     anim.save('unused.null', writer=writer)
     assert len(frames_generated) == 5
     for f in frames_generated:
-        assert weakref_assertion_fn(f())
+        # If cache_frame_data is True, then the weakref should be alive;
+        # if cache_frame_data is False, then the weakref should be dead (None).
+        assert (f() is None) != cache_frame_data
