diff --git a/doc/api/next_api_changes/2019-03-03-AL.rst b/doc/api/next_api_changes/2019-03-03-AL.rst new file mode 100644 index 000000000000..7be2d21fb808 --- /dev/null +++ b/doc/api/next_api_changes/2019-03-03-AL.rst @@ -0,0 +1,14 @@ +Deprecations +```````````` + +The following methods and attributes of the `MovieWriterRegistry` class are +deprecated: ``set_dirty``, ``ensure_not_dirty``, ``reset_available_writers``, +``avail``. + +The ``rcsetup.validate_animation_writer_path`` function is deprecated. + +`MovieWriterRegistry` now always checks the availability of the writer classes +before returning them. If one wishes, for example, to get the first available +writer, without performing the availability check on subsequent writers, it is +now possible to iterate over the registry, which will yield the names of the +available classes. diff --git a/lib/matplotlib/animation.py b/lib/matplotlib/animation.py index 1f3d8b2dee4d..a622827e05cf 100644 --- a/lib/matplotlib/animation.py +++ b/lib/matplotlib/animation.py @@ -98,13 +98,11 @@ def correct_roundoff(x, dpi, n): class MovieWriterRegistry(object): '''Registry of available writer classes by human readable name.''' def __init__(self): - self.avail = dict() self._registered = dict() - self._dirty = False + @cbook.deprecated("3.2") def set_dirty(self): """Sets a flag to re-setup the writers.""" - self._dirty = True def register(self, name): """Decorator for registering a class under a name. @@ -115,32 +113,27 @@ def register(self, name): class Foo: pass """ - def wrapper(writerClass): - self._registered[name] = writerClass - if writerClass.isAvailable(): - self.avail[name] = writerClass - return writerClass + def wrapper(writer_cls): + self._registered[name] = writer_cls + return writer_cls return wrapper + @cbook.deprecated("3.2") def ensure_not_dirty(self): """If dirty, reasks the writers if they are available""" - if self._dirty: - self.reset_available_writers() + @cbook.deprecated("3.2") def reset_available_writers(self): """Reset the available state of all registered writers""" - self.avail = {name: writerClass - for name, writerClass in self._registered.items() - if writerClass.isAvailable()} - self._dirty = False - def list(self): - '''Get a list of available MovieWriters.''' - self.ensure_not_dirty() - return list(self.avail) + @cbook.deprecated("3.2") + @property + def avail(self): + return {name: self._registered[name] for name in self.list()} def is_available(self, name): - '''Check if given writer is available by name. + """ + Check if given writer is available by name. Parameters ---------- @@ -149,19 +142,28 @@ def is_available(self, name): Returns ------- available : bool - ''' - self.ensure_not_dirty() - return name in self.avail - - def __getitem__(self, name): - self.ensure_not_dirty() - if not self.avail: - raise RuntimeError("No MovieWriters available!") + """ try: - return self.avail[name] + cls = self._registered[name] except KeyError: - raise RuntimeError( - 'Requested MovieWriter ({}) not available'.format(name)) + return False + return cls.isAvailable() + + def __iter__(self): + """Iterate over names of available writer class.""" + for name in self._registered: + if self.is_available(name): + yield name + + def list(self): + """Get a list of available MovieWriters.""" + return [*self] + + def __getitem__(self, name): + """Get an available writer class from its name.""" + if self.is_available(name): + return self._registered[name] + raise RuntimeError(f"Requested MovieWriter ({name}) not available") writers = MovieWriterRegistry() @@ -1071,22 +1073,21 @@ class to use, such as 'ffmpeg'. If ``None``, defaults to # If we have the name of a writer, instantiate an instance of the # registered class. if isinstance(writer, str): - if writer in writers.avail: + if writers.is_available(writer): writer = writers[writer](fps, codec, bitrate, extra_args=extra_args, metadata=metadata) else: - if writers.list(): - alt_writer = writers[writers.list()[0]] - _log.warning("MovieWriter %s unavailable; trying to use " - "%s instead.", writer, alt_writer) - writer = alt_writer( - fps, codec, bitrate, - extra_args=extra_args, metadata=metadata) - else: + alt_writer = next(writers, None) + if alt_writer is None: raise ValueError("Cannot save animation: no writers are " "available. Please install ffmpeg to " "save animations.") + _log.warning("MovieWriter %s unavailable; trying to use %s " + "instead.", writer, alt_writer) + writer = alt_writer( + fps, codec, bitrate, + extra_args=extra_args, metadata=metadata) _log.info('Animation.save using %s', type(writer)) if 'bbox_inches' in savefig_kwargs: diff --git a/lib/matplotlib/rcsetup.py b/lib/matplotlib/rcsetup.py index dd1f08120038..580fd2140bc1 100644 --- a/lib/matplotlib/rcsetup.py +++ b/lib/matplotlib/rcsetup.py @@ -927,6 +927,7 @@ def validate_hist_bins(s): " a sequence of floats".format(valid_strs)) +@cbook.deprecated("3.2") def validate_animation_writer_path(p): # Make sure it's a string and then figure out if the animations # are already loaded and reset the writers (which will validate @@ -1454,15 +1455,15 @@ def _validate_linestyle(ls): # Additional arguments for HTML writer 'animation.html_args': [[], validate_stringlist], # Path to ffmpeg binary. If just binary name, subprocess uses $PATH. - 'animation.ffmpeg_path': ['ffmpeg', validate_animation_writer_path], + 'animation.ffmpeg_path': ['ffmpeg', validate_string], # Additional arguments for ffmpeg movie writer (using pipes) 'animation.ffmpeg_args': [[], validate_stringlist], # Path to AVConv binary. If just binary name, subprocess uses $PATH. - 'animation.avconv_path': ['avconv', validate_animation_writer_path], + 'animation.avconv_path': ['avconv', validate_string], # Additional arguments for avconv movie writer (using pipes) 'animation.avconv_args': [[], validate_stringlist], # Path to convert binary. If just binary name, subprocess uses $PATH. - 'animation.convert_path': ['convert', validate_animation_writer_path], + 'animation.convert_path': ['convert', validate_string], # Additional arguments for convert movie writer (using pipes) 'animation.convert_args': [[], validate_stringlist], diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py index d587271d5a91..67cdc42f9dcd 100644 --- a/lib/matplotlib/tests/test_animation.py +++ b/lib/matplotlib/tests/test_animation.py @@ -187,27 +187,14 @@ def test_no_length_frames(): def test_movie_writer_registry(): - ffmpeg_path = mpl.rcParams['animation.ffmpeg_path'] - # Not sure about the first state as there could be some writer - # which set rcparams - # assert not animation.writers._dirty assert len(animation.writers._registered) > 0 - animation.writers.list() # resets dirty state - assert not animation.writers._dirty mpl.rcParams['animation.ffmpeg_path'] = "not_available_ever_xxxx" - assert animation.writers._dirty - animation.writers.list() # resets - assert not animation.writers._dirty assert not animation.writers.is_available("ffmpeg") # something which is guaranteed to be available in path # and exits immediately bin = "true" if sys.platform != 'win32' else "where" mpl.rcParams['animation.ffmpeg_path'] = bin - assert animation.writers._dirty - animation.writers.list() # resets - assert not animation.writers._dirty assert animation.writers.is_available("ffmpeg") - mpl.rcParams['animation.ffmpeg_path'] = ffmpeg_path @pytest.mark.skipif( @@ -248,18 +235,14 @@ def test_failing_ffmpeg(tmpdir, monkeypatch): succeeds when called with no arguments (so that it gets registered by `isAvailable`), but fails otherwise, and add it to the $PATH. """ - try: - with tmpdir.as_cwd(): - monkeypatch.setenv("PATH", ".:" + os.environ["PATH"]) - exe_path = Path(str(tmpdir), "ffmpeg") - exe_path.write_text("#!/bin/sh\n" - "[[ $@ -eq 0 ]]\n") - os.chmod(str(exe_path), 0o755) - animation.writers.reset_available_writers() - with pytest.raises(subprocess.CalledProcessError): - make_animation().save("test.mpeg") - finally: - animation.writers.reset_available_writers() + with tmpdir.as_cwd(): + monkeypatch.setenv("PATH", ".:" + os.environ["PATH"]) + exe_path = Path(str(tmpdir), "ffmpeg") + exe_path.write_text("#!/bin/sh\n" + "[[ $@ -eq 0 ]]\n") + os.chmod(str(exe_path), 0o755) + with pytest.raises(subprocess.CalledProcessError): + make_animation().save("test.mpeg") @pytest.mark.parametrize("cache_frame_data, weakref_assertion_fn", [