From e12f16abe754fe6ed4ee388817b33be51177307f Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Tue, 1 Oct 2019 18:13:34 +0200 Subject: [PATCH 1/2] Deprecate JPEG-specific kwargs and rcParams to savefig. Saving Matplotlib figures to jpeg is generally not a great idea to start with (even at the default quality of 95 there are visible (faint) artefacts around sharp lines, and at quality=95 the files produced are bigger than the corresponding pngs anyways). We don't need to completely get rid of jpeg support, but we can at least simplify the code path (which otherwise also needs to be duplicated in mplcairo) and not have to document these jpeg-specific kwargs (in two places!). Note that users can still set them via `pil_kwargs`. The changes to _delete_parameter are so that we can write ``` @_delete_parameter(..., "foo") def f(**kwargs): ... ``` where `foo` actually only shows up in `kwargs`. --- doc/api/next_api_changes/deprecations.rst | 10 +++- lib/matplotlib/__init__.py | 1 + lib/matplotlib/backends/backend_agg.py | 24 ++++++++-- lib/matplotlib/backends/backend_wx.py | 5 +- lib/matplotlib/cbook/deprecation.py | 47 ++++++++++++------- lib/matplotlib/figure.py | 6 +++ .../mpl-data/stylelib/classic.mplstyle | 1 - matplotlibrc.template | 1 - 8 files changed, 71 insertions(+), 24 deletions(-) diff --git a/doc/api/next_api_changes/deprecations.rst b/doc/api/next_api_changes/deprecations.rst index e9f50c8d84ab..0a7c9ce26182 100644 --- a/doc/api/next_api_changes/deprecations.rst +++ b/doc/api/next_api_changes/deprecations.rst @@ -225,7 +225,6 @@ The following validators, defined in `.rcsetup`, are deprecated: ``validate_movie_frame_fmt``, ``validate_axis_locator``, ``validate_movie_html_fmt``, ``validate_grid_axis``, ``validate_axes_titlelocation``, ``validate_toolbar``, -<<<<<<< HEAD ``validate_ps_papersize``, ``validate_legend_loc``, ``validate_bool_maybe_none``, ``validate_hinting``, ``validate_movie_writers``. @@ -299,3 +298,12 @@ is deprecated, set the offset to 0 instead. ``autofmt_xdate(which=None)`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This is deprecated, use its more explicit synonym, ``which="major"``, instead. + +JPEG options +~~~~~~~~~~~~ +The *quality*, *optimize*, and *progressive* keyword arguments to +`~.Figure.savefig`, which were only used when saving to JPEG, are deprecated. +:rc:`savefig.jpeg_quality` is likewise deprecated. + +Such options should now be directly passed to Pillow using +``savefig(..., pil_kwargs={"quality": ..., "optimize": ..., "progressive": ...})``. diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py index 7eb23fc22d72..1be0aebc6414 100644 --- a/lib/matplotlib/__init__.py +++ b/lib/matplotlib/__init__.py @@ -604,6 +604,7 @@ def gen_candidates(): 'animation.avconv_args': ('3.3',), 'mathtext.fallback_to_cm': ('3.3',), 'keymap.all_axes': ('3.3',), + 'savefig.jpeg_quality': ('3.3',), } diff --git a/lib/matplotlib/backends/backend_agg.py b/lib/matplotlib/backends/backend_agg.py index c4cc22d24a3e..2226104db981 100644 --- a/lib/matplotlib/backends/backend_agg.py +++ b/lib/matplotlib/backends/backend_agg.py @@ -512,6 +512,12 @@ def print_to_buffer(self): # matches the dpi kwarg (if any). @cbook._delete_parameter("3.2", "dryrun") + @cbook._delete_parameter("3.3", "quality", + alternative="pil_kwargs={'quality': ...}") + @cbook._delete_parameter("3.3", "optimize", + alternative="pil_kwargs={'optimize': ...}") + @cbook._delete_parameter("3.3", "progressive", + alternative="pil_kwargs={'progressive': ...}") def print_jpg(self, filename_or_obj, *args, dryrun=False, pil_kwargs=None, **kwargs): """ @@ -528,14 +534,17 @@ def print_jpg(self, filename_or_obj, *args, dryrun=False, pil_kwargs=None, The image quality, on a scale from 1 (worst) to 95 (best). Values above 95 should be avoided; 100 disables portions of the JPEG compression algorithm, and results in large files - with hardly any gain in image quality. + with hardly any gain in image quality. This parameter is + deprecated. optimize : bool, default: False Whether the encoder should make an extra pass over the image - in order to select optimal encoder settings. + in order to select optimal encoder settings. This parameter is + deprecated. progressive : bool, default: False Whether the image should be stored as a progressive JPEG file. + This parameter is deprecated. pil_kwargs : dict, optional Additional keyword arguments that are passed to @@ -555,7 +564,16 @@ def print_jpg(self, filename_or_obj, *args, dryrun=False, pil_kwargs=None, for k in ["quality", "optimize", "progressive"]: if k in kwargs: pil_kwargs.setdefault(k, kwargs[k]) - pil_kwargs.setdefault("quality", mpl.rcParams["savefig.jpeg_quality"]) + if "quality" not in pil_kwargs: + quality = pil_kwargs["quality"] = \ + dict.__getitem__(mpl.rcParams, "savefig.jpeg_quality") + if quality not in [0, 75, 95]: # default qualities. + cbook.warn_deprecated( + "3.3", name="savefig.jpeg_quality", obj_type="rcParam", + addendum="Set the quality using " + "`pil_kwargs={'quality': ...}`; the future default " + "quality will be 75, matching the default of Pillow and " + "libjpeg.") pil_kwargs.setdefault("dpi", (self.figure.dpi, self.figure.dpi)) return background.save( filename_or_obj, format='jpeg', **pil_kwargs) diff --git a/lib/matplotlib/backends/backend_wx.py b/lib/matplotlib/backends/backend_wx.py index d23961099740..a01f57aa2bfe 100644 --- a/lib/matplotlib/backends/backend_wx.py +++ b/lib/matplotlib/backends/backend_wx.py @@ -885,8 +885,9 @@ def _print_image(self, filename, filetype, *args, **kwargs): # are saving a JPEG, convert the wx.Bitmap to a wx.Image, # and set the quality. if filetype == wx.BITMAP_TYPE_JPEG: - jpeg_quality = kwargs.get('quality', - mpl.rcParams['savefig.jpeg_quality']) + jpeg_quality = kwargs.get( + 'quality', + dict.__getitem__(mpl.rcParams, 'savefig.jpeg_quality')) image = self.bitmap.ConvertToImage() image.SetOption(wx.IMAGE_OPTION_QUALITY, str(jpeg_quality)) diff --git a/lib/matplotlib/cbook/deprecation.py b/lib/matplotlib/cbook/deprecation.py index 06faec380eac..f6c8bfa6473d 100644 --- a/lib/matplotlib/cbook/deprecation.py +++ b/lib/matplotlib/cbook/deprecation.py @@ -312,7 +312,8 @@ def _delete_parameter(since, name, func=None, **kwargs): Decorator indicating that parameter *name* of *func* is being deprecated. The actual implementation of *func* should keep the *name* parameter in its - signature. + signature, or accept a ``**kwargs`` argument (through which *name* would be + passed). Parameters that come after the deprecated parameter effectively become keyword-only (as they cannot be passed positionally without triggering the @@ -320,7 +321,8 @@ def _delete_parameter(since, name, func=None, **kwargs): such after the deprecation period has passed and the deprecated parameter is removed. - Additional keyword arguments are passed to `.warn_deprecated`. + Parameters other than *since*, *name*, and *func* are keyword-only and + forwarded to `.warn_deprecated`. Examples -------- @@ -334,17 +336,25 @@ def func(used_arg, other_arg, unused, more_args): ... return functools.partial(_delete_parameter, since, name, **kwargs) signature = inspect.signature(func) - assert name in signature.parameters, ( - f"Matplotlib internal error: {name!r} must be a parameter for " - f"{func.__name__}()") - kind = signature.parameters[name].kind - is_varargs = kind is inspect.Parameter.VAR_POSITIONAL - is_varkwargs = kind is inspect.Parameter.VAR_KEYWORD - if not is_varargs and not is_varkwargs: - func.__signature__ = signature = signature.replace(parameters=[ - param.replace(default=_deprecated_parameter) if param.name == name - else param - for param in signature.parameters.values()]) + # Name of `**kwargs` parameter of the decorated function, typically + # "kwargs" if such a parameter exists, or None if the decorated function + # doesn't accept `**kwargs`. + kwargs_name = next((param.name for param in signature.parameters.values() + if param.kind == inspect.Parameter.VAR_KEYWORD), None) + if name in signature.parameters: + kind = signature.parameters[name].kind + is_varargs = kind is inspect.Parameter.VAR_POSITIONAL + is_varkwargs = kind is inspect.Parameter.VAR_KEYWORD + if not is_varargs and not is_varkwargs: + func.__signature__ = signature = signature.replace(parameters=[ + param.replace(default=_deprecated_parameter) + if param.name == name else param + for param in signature.parameters.values()]) + else: + is_varargs = is_varkwargs = False + assert kwargs_name, ( + f"Matplotlib internal error: {name!r} must be a parameter for " + f"{func.__name__}()") @functools.wraps(func) def wrapper(*inner_args, **inner_kwargs): @@ -361,13 +371,18 @@ def wrapper(*inner_args, **inner_kwargs): f"support for them will be removed %(removal)s.") # We cannot just check `name not in arguments` because the pyplot # wrappers always pass all arguments explicitly. - elif name in arguments and arguments[name] != _deprecated_parameter: + elif any(name in d and d[name] != _deprecated_parameter + for d in [arguments, arguments.get(kwargs_name, {})]): + addendum = (f"If any parameter follows {name!r}, they should be " + f"passed as keyword, not positionally.") + if kwargs.get("addendum"): + kwargs["addendum"] += " " + addendum + else: + kwargs["addendum"] = addendum warn_deprecated( since, name=repr(name), obj_type=f"parameter of {func.__name__}()", - addendum=(f"If any parameter follows {name!r}, they should be " - f"passed as keyword, not positionally."), **kwargs) return func(*inner_args, **inner_kwargs) diff --git a/lib/matplotlib/figure.py b/lib/matplotlib/figure.py index 16a940aa86b3..65c2bba2a126 100644 --- a/lib/matplotlib/figure.py +++ b/lib/matplotlib/figure.py @@ -2065,17 +2065,23 @@ def savefig(self, fname, *, transparent=None, **kwargs): the JPEG compression algorithm, and results in large files with hardly any gain in image quality. + This parameter is deprecated. + optimize : bool, default: False Applicable only if *format* is 'jpg' or 'jpeg', ignored otherwise. Whether the encoder should make an extra pass over the image in order to select optimal encoder settings. + This parameter is deprecated. + progressive : bool, default: False Applicable only if *format* is 'jpg' or 'jpeg', ignored otherwise. Whether the image should be stored as a progressive JPEG file. + This parameter is deprecated. + facecolor : color, default: :rc:`savefig.facecolor` The facecolor of the figure. diff --git a/lib/matplotlib/mpl-data/stylelib/classic.mplstyle b/lib/matplotlib/mpl-data/stylelib/classic.mplstyle index d9038b6b63b9..ef5dfa8e3f2a 100644 --- a/lib/matplotlib/mpl-data/stylelib/classic.mplstyle +++ b/lib/matplotlib/mpl-data/stylelib/classic.mplstyle @@ -420,7 +420,6 @@ savefig.bbox : standard # 'tight' or 'standard'. # e.g. setting animation.writer to ffmpeg will not work, # use ffmpeg_file instead savefig.pad_inches : 0.1 # Padding to be used when bbox is set to 'tight' -savefig.jpeg_quality: 95 # when a jpeg is saved, the default quality parameter. savefig.transparent : False # setting that controls whether figures are saved with a # transparent background by default savefig.orientation : portrait diff --git a/matplotlibrc.template b/matplotlibrc.template index 2dbe7bbb9b29..c323da5c59f7 100644 --- a/matplotlibrc.template +++ b/matplotlibrc.template @@ -651,7 +651,6 @@ # e.g. setting animation.writer to ffmpeg will not work, # use ffmpeg_file instead #savefig.pad_inches: 0.1 # Padding to be used when bbox is set to 'tight' -#savefig.jpeg_quality: 95 # when a jpeg is saved, the default quality parameter. #savefig.directory: ~ # default directory in savefig dialog box, # leave empty to always use current working directory #savefig.transparent: False # setting that controls whether figures are saved with a From 6a66d30e90f1bef924263cd6ed47b6239a79f07a Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Wed, 12 Feb 2020 00:07:33 +0100 Subject: [PATCH 2/2] Add tests for delete_parameter. --- lib/matplotlib/tests/test_cbook.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py index 12b6edde1fc8..46cf671c042e 100644 --- a/lib/matplotlib/tests/test_cbook.py +++ b/lib/matplotlib/tests/test_cbook.py @@ -574,6 +574,28 @@ def test_safe_first_element_pandas_series(pd): assert actual == 0 +def test_delete_parameter(): + @cbook._delete_parameter("3.0", "foo") + def func1(foo=None): + pass + + @cbook._delete_parameter("3.0", "foo") + def func2(**kwargs): + pass + + for func in [func1, func2]: + func() # No warning. + with pytest.warns(MatplotlibDeprecationWarning): + func(foo="bar") + + def pyplot_wrapper(foo=cbook.deprecation._deprecated_parameter): + func1(foo) + + pyplot_wrapper() # No warning. + with pytest.warns(MatplotlibDeprecationWarning): + func(foo="bar") + + def test_make_keyword_only(): @cbook._make_keyword_only("3.0", "arg") def func(pre, arg, post=None):