Don't use stdlib private API in animation.py.

anntzer · anntzer · commit 02e5358d41d0 · 2018-10-02T17:14:31.000+02:00
The _std{out,err}_buff attributes on Popen objects don't exist anymore
on Unices (since CPython 3.4).  As these buffers are not accessible
anymore in finish(), move the status check and possible exception
throwing to cleanup(), where they are still available.

Don't silently drop exceptions; this shows that ProcessStandin
previously used by HTMLWriter was also buggy (communicate() needed to
return bytes, not str); instead of using it, just do all the work in
finish(), similarly to PillowWriter).
diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py
@@ -19,7 +19,7 @@
 import abc
 import base64
 import contextlib
-from io import BytesIO
+from io import BytesIO, TextIOWrapper
 import itertools
 import logging
 import os
@@ -346,12 +346,11 @@ def _run(self):
         # movie file.  *args* returns the sequence of command line arguments
         # from a few configuration options.
         command = self._args()
-        output = subprocess.PIPE
         _log.info('MovieWriter.run: running command: %s', command)
-        self._proc = subprocess.Popen(command, shell=False,
-                                      stdout=output, stderr=output,
-                                      stdin=subprocess.PIPE,
-                                      creationflags=subprocess_creation_flags)
+        PIPE = subprocess.PIPE
+        self._proc = subprocess.Popen(
+            command, stdin=PIPE, stdout=PIPE, stderr=PIPE,
+            creationflags=subprocess_creation_flags)
 
     def finish(self):
         '''Finish any processing for writing the movie.'''
@@ -394,8 +393,18 @@ def cleanup(self):
         '''Clean-up and collect the process used to write the movie file.'''
         out, err = self._proc.communicate()
         self._frame_sink().close()
-        _log.debug('MovieWriter -- Command stdout:\n%s', out)
-        _log.debug('MovieWriter -- Command stderr:\n%s', err)
+        # Use the encoding/errors that universal_newlines would use.
+        out = TextIOWrapper(BytesIO(out)).read()
+        err = TextIOWrapper(BytesIO(err)).read()
+        _log.debug("MovieWriter stdout:\n%s", out)
+        _log.debug("MovieWriter stderr:\n%s", err)
+        if self._proc.returncode:
+            raise RuntimeError('Error creating movie, return code {}\n'
+                               'stdout:\n'
+                               '{}\n'
+                               'stderr:\n'
+                               '{}\n'
+                               .format(self._proc.returncode, out, err))
 
     @classmethod
     def bin_path(cls):
@@ -520,19 +529,6 @@ def finish(self):
         self._run()
         MovieWriter.finish(self)  # Will call clean-up
 
-        # Check error code for creating file here, since we just run
-        # the process here, rather than having an open pipe.
-        if self._proc.returncode:
-            try:
-                stdout = [s.decode() for s in self._proc._stdout_buff]
-                stderr = [s.decode() for s in self._proc._stderr_buff]
-                _log.info("MovieWriter.finish: stdout: %s", stdout)
-                _log.info("MovieWriter.finish: stderr: %s", stderr)
-            except Exception as e:
-                pass
-            raise RuntimeError('Error creating movie, return code: {}'
-                               .format(self._proc.returncode))
-
     def cleanup(self):
         MovieWriter.cleanup(self)
 
@@ -894,17 +890,7 @@ def grab_frame(self, **savefig_kwargs):
         else:
             return super().grab_frame(**savefig_kwargs)
 
-    def _run(self):
-        # make a duck-typed subprocess stand in
-        # this is called by the MovieWriter base class, but not used here.
-        class ProcessStandin(object):
-            returncode = 0
-
-            def communicate(self):
-                return '', ''
-
-        self._proc = ProcessStandin()
-
+    def finish(self):
         # save the frames to an html file
         if self.embed_frames:
             fill_frames = _embedded_frames(self._saved_frames,
