Merge pull request #18902 from anntzer/imv

jklymak · web-flow · commit bba5c765f750 · 2020-11-09T10:00:04.000-08:00
Move ImageMagick version exclusion to _get_executable_info.
diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py
@@ -324,7 +324,6 @@ def impl(args, regex, min_ver=None, ignore_exit_code=False):
         # try without it:
         return impl(["inkscape", "-V"], "Inkscape ([^ ]*)")
     elif name == "magick":
-        path = None
         if sys.platform == "win32":
             # Check the registry to avoid confusing ImageMagick's convert with
             # Windows's builtin convert.exe.
@@ -339,18 +338,24 @@ def impl(args, regex, min_ver=None, ignore_exit_code=False):
                         binpath = winreg.QueryValueEx(hkey, "BinPath")[0]
                 except OSError:
                     pass
+            path = None
             if binpath:
                 for name in ["convert.exe", "magick.exe"]:
                     candidate = Path(binpath, name)
                     if candidate.exists():
                         path = str(candidate)
                         break
+            if path is None:
+                raise ExecutableNotFoundError(
+                    "Failed to find an ImageMagick installation")
         else:
             path = "convert"
-        if path is None:
+        info = impl([path, "--version"], r"^Version: ImageMagick (\S*)")
+        if info.version == "7.0.10-34":
+            # https://github.com/ImageMagick/ImageMagick/issues/2720
             raise ExecutableNotFoundError(
-                "Failed to find an ImageMagick installation")
-        return impl([path, "--version"], r"^Version: ImageMagick (\S*)")
+                f"You have ImageMagick {info.version}, which is unsupported")
+        return info
     elif name == "pdftops":
         info = impl(["pdftops", "-v"], "^pdftops version (.*)",
                     ignore_exit_code=True)
diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py
@@ -191,11 +191,6 @@ def test_save_animation_smoketest(tmpdir, writer, output, anim):
 def test_animation_repr_html(writer, html, want, anim):
     # create here rather than in the fixture otherwise we get __del__ warnings
     # about producing no output
-    if writer == 'imagemagick':
-        ret = subprocess.run(['convert', '--version'], capture_output=True)
-        if b'7.0.10-34' in ret.stdout:
-            # see https://github.com/ImageMagick/ImageMagick/issues/2720
-            pytest.skip("Known bad version of imagemagick")
     anim = animation.FuncAnimation(**anim)
     with plt.rc_context({'animation.writer': writer,
                          'animation.html': html}):
