Merge pull request #197 from astrofrog/deterministic-default

Author: ConorMacBride

docs/configuration.rst

@@ -250,8 +250,8 @@ If the RMS difference is greater than the tolerance, the test will fail.
 Whether to make metadata deterministic
 --------------------------------------
 | **kwarg**: ``deterministic=<bool>``
-| **CLI**: ---
-| **INI**: ---
+| **CLI**: ``--mpl-deterministic`` or ``--mpl-no-deterministic``
+| **INI**: ``mpl-deterministic = <bool>``
 | Default: ``True`` (PNG: ``False``)
 
 Whether to make the image file metadata deterministic.
@@ -270,6 +270,11 @@ By default, ``pytest-mpl`` will save and compare figures in PNG format.
 However, it is possible to set the format to use by setting, e.g., ``savefig_kwargs={"format": "pdf"}`` when configuring the :ref:`savefig_kwargs configuration option <savefig-kwargs>`.
 Note that Ghostscript is required to be installed for comparing PDF and EPS figures, while Inkscape is required for SVG comparison.
 
+.. note::
+
+    A future major release of ``pytest-mpl`` will generate deterministic PNG files by default.
+    It is recommended to explicitly set this configuration option to avoid hashes changing.
+
 Whether to remove titles and axis tick labels
 ---------------------------------------------
 | **kwargs**: ``remove_text=<bool>``

pytest_mpl/plugin.py

@@ -203,6 +203,13 @@ def pytest_addoption(parser):
     group.addoption(f"--{option}", help=msg, action="store")
     parser.addini(option, help=msg)
 
+    msg = "whether to make the image file metadata deterministic"
+    option_true = "mpl-deterministic"
+    option_false = "mpl-no-deterministic"
+    group.addoption(f"--{option_true}", help=msg, action="store_true")
+    group.addoption(f"--{option_false}", help=msg, action="store_true")
+    parser.addini(option_true, help=msg, type="bool", default=None)
+
     msg = "default backend to use for tests, unless specified in the mpl_image_compare decorator"
     option = "mpl-default-backend"
     group.addoption(f"--{option}", help=msg, action="store")
@@ -244,6 +251,21 @@ def get_cli_or_ini(name, default=None):
                 default_tolerance = int(default_tolerance)
             else:
                 default_tolerance = float(default_tolerance)
+
+        deterministic_ini = config.getini("mpl-deterministic")
+        deterministic_flag_true = config.getoption("--mpl-deterministic")
+        deterministic_flag_false = config.getoption("--mpl-no-deterministic")
+        if deterministic_flag_true and deterministic_flag_false:
+            raise ValueError("Only one of `--mpl-deterministic` and `--mpl-no-deterministic` can be set.")
+        if deterministic_flag_true:
+            deterministic = True
+        elif deterministic_flag_false:
+            deterministic = False
+        elif isinstance(deterministic_ini, bool):
+            deterministic = deterministic_ini
+        else:
+            deterministic = None
+
         default_style = get_cli_or_ini("mpl-default-style", DEFAULT_STYLE)
         default_backend = get_cli_or_ini("mpl-default-backend", DEFAULT_BACKEND)
 
@@ -279,6 +301,7 @@ def get_cli_or_ini(name, default=None):
             use_full_test_name=use_full_test_name,
             default_style=default_style,
             default_tolerance=default_tolerance,
+            deterministic=deterministic,
             default_backend=default_backend,
             _hash_library_from_cli=_hash_library_from_cli,
         )
@@ -341,6 +364,7 @@ def __init__(
         use_full_test_name=False,
         default_style=DEFAULT_STYLE,
         default_tolerance=DEFAULT_TOLERANCE,
+        deterministic=None,
         default_backend=DEFAULT_BACKEND,
         _hash_library_from_cli=False,  # for backwards compatibility
     ):
@@ -367,6 +391,7 @@ def __init__(
 
         self.default_style = default_style
         self.default_tolerance = default_tolerance
+        self.deterministic = deterministic
         self.default_backend = default_backend
 
         # Generate the containing dir for all test results
@@ -639,12 +664,45 @@ def save_figure(self, item, fig, filename):
             filename = str(filename)
         compare = get_compare(item)
         savefig_kwargs = compare.kwargs.get('savefig_kwargs', {})
-        deterministic = compare.kwargs.get('deterministic', False)
+        deterministic = compare.kwargs.get('deterministic', self.deterministic)
 
         original_source_date_epoch = os.environ.get('SOURCE_DATE_EPOCH', None)
 
         extra_rcparams = {}
 
+        ext = self._file_extension(item)
+
+        if deterministic is None:
+
+            # The deterministic option should only matter for hash-based tests,
+            # so we first check if a hash library is being used
+
+            if self.hash_library or compare.kwargs.get('hash_library', None):
+
+                if ext == 'png':
+                    if 'metadata' not in savefig_kwargs or 'Software' not in savefig_kwargs['metadata']:
+                        warnings.warn("deterministic option not set (currently defaulting to False), "
+                                      "in future this will default to True to give consistent "
+                                      "hashes across Matplotlib versions. To suppress this warning, "
+                                      "set deterministic to True if you are happy with the future "
+                                      "behavior or to False if you want to preserve the old behavior.",
+                                      FutureWarning)
+                    else:
+                        # Set to False but in practice because Software is set to a constant value
+                        # by the caller, the output will be deterministic (we don't want to change
+                        # Software to None if the caller set it to e.g. 'test')
+                        deterministic = False
+                else:
+                    deterministic = True
+
+            else:
+
+                # We can just default to True since it shouldn't matter and in
+                # case generated images are somehow used in future to compute
+                # hashes
+
+                deterministic = True
+
         if deterministic:
 
             # Make sure we don't modify the original dictionary in case is a common
@@ -654,8 +712,6 @@ def save_figure(self, item, fig, filename):
             if 'metadata' not in savefig_kwargs:
                 savefig_kwargs['metadata'] = {}
 
-            ext = self._file_extension(item)
-
             if ext == 'png':
                 extra_metadata = {"Software": None}
             elif ext == 'pdf':

tests/helpers.py

@@ -1,7 +1,36 @@
+import sys
 from pathlib import Path
 
+import matplotlib
+import pytest
+from matplotlib.testing.compare import converter
+from packaging.version import Version
+
+MPL_VERSION = Version(matplotlib.__version__)
+
 
 def pytester_path(pytester):
     if hasattr(pytester, "path"):
         return pytester.path
     return Path(pytester.tmpdir)  # pytest v5
+
+
+def skip_if_format_unsupported(file_format, using_hashes=False):
+    if file_format == 'svg' and MPL_VERSION < Version('3.3'):
+        pytest.skip('SVG comparison is only supported in Matplotlib 3.3 and above')
+
+    if using_hashes:
+
+        if file_format == 'pdf' and MPL_VERSION < Version('2.1'):
+            pytest.skip('PDF hashes are only deterministic in Matplotlib 2.1 and above')
+        elif file_format == 'eps' and MPL_VERSION < Version('2.1'):
+            pytest.skip('EPS hashes are only deterministic in Matplotlib 2.1 and above')
+
+    if using_hashes and not sys.platform.startswith('linux'):
+        pytest.skip('Hashes for vector graphics are only provided in the hash library for Linux')
+
+    if file_format != 'png' and file_format not in converter:
+        if file_format == 'svg':
+            pytest.skip('Comparing SVG files requires inkscape to be installed')
+        else:
+            pytest.skip('Comparing EPS and PDF files requires ghostscript to be installed')

tests/test_deterministic.py

@@ -0,0 +1,128 @@
+import matplotlib
+import matplotlib.pyplot as plt
+import pytest
+from helpers import pytester_path, skip_if_format_unsupported
+from packaging.version import Version
+from PIL import Image
+
+MPL_VERSION = Version(matplotlib.__version__)
+
+METADATA = {
+    "png": {"Software": None},
+    "pdf": {"Creator": None, "Producer": None, "CreationDate": None},
+    "eps": {"Creator": "test"},
+    "svg": {"Date": None},
+}
+
+
+def test_multiple_cli_flags(pytester):
+    result = pytester.runpytest("--mpl", "--mpl-deterministic", "--mpl-no-deterministic")
+    result.stderr.fnmatch_lines(
+        ["*ValueError: Only one of `--mpl-deterministic` and `--mpl-no-deterministic` can be set.*"]
+    )
+
+
+def test_warning(pytester):
+    path = pytester_path(pytester)
+    hash_library = path / "hash_library.json"
+    kwarg = f"hash_library=r'{hash_library}'"
+    pytester.makepyfile(
+        f"""
+        import matplotlib.pyplot as plt
+        import pytest
+        @pytest.mark.mpl_image_compare({kwarg})
+        def test_mpl():
+            fig, ax = plt.subplots()
+            ax.plot([1, 3, 2])
+            return fig
+        """
+    )
+    result = pytester.runpytest(f"--mpl-generate-hash-library={hash_library}")
+    result.stdout.fnmatch_lines(["*FutureWarning: deterministic option not set*"])
+    result.assert_outcomes(failed=1)
+
+
+@pytest.mark.parametrize("file_format", ["eps", "pdf", "png", "svg"])
+@pytest.mark.parametrize(
+    "ini, cli, kwarg, success_expected",
+    [
+        ("true", "", None, True),
+        ("false", "--mpl-deterministic", None, True),
+        ("true", "--mpl-no-deterministic", None, False),
+        ("", "--mpl-no-deterministic", True, True),
+        ("true", "", False, False),
+    ],
+)
+@pytest.mark.skipif(MPL_VERSION < Version("3.3.0"), reason="Test unsupported: Default metadata is different in MPL<3.3")
+def test_config(pytester, file_format, ini, cli, kwarg, success_expected):
+    skip_if_format_unsupported(file_format, using_hashes=True)
+
+    path = pytester_path(pytester)
+    baseline_dir = path / "baseline"
+    hash_library = path / "hash_library.json"
+
+    ini = f"mpl-deterministic = {ini}" if ini else ""
+    pytester.makeini(
+        f"""
+        [pytest]
+        mpl-hash-library = {hash_library}
+        {ini}
+        """
+    )
+
+    kwarg = f", deterministic={kwarg}" if isinstance(kwarg, bool) else ""
+    pytester.makepyfile(
+        f"""
+        import matplotlib.pyplot as plt
+        import pytest
+        @pytest.mark.mpl_image_compare(savefig_kwargs={{'format': '{file_format}'}}{kwarg})
+        def test_mpl():
+            fig, ax = plt.subplots()
+            ax.plot([1, 2, 3])
+            return fig
+        """
+    )
+
+    # Generate baseline hashes
+    assert not hash_library.exists()
+    pytester.runpytest(
+        f"--mpl-generate-path={baseline_dir}",
+        f"--mpl-generate-hash-library={hash_library}",
+        cli,
+    )
+    assert hash_library.exists()
+    baseline_image = baseline_dir / f"test_mpl.{file_format}"
+    assert baseline_image.exists()
+    deterministic_metadata = METADATA[file_format]
+
+    if file_format == "svg":  # The only format that is reliably non-deterministic between runs
+        result = pytester.runpytest("--mpl", f"--mpl-baseline-path={baseline_dir}", cli)
+        if success_expected:
+            result.assert_outcomes(passed=1)
+        else:
+            result.assert_outcomes(failed=1)
+
+    elif file_format == "pdf":
+        with open(baseline_image, "rb") as fp:
+            file = str(fp.read())
+        for metadata_key in deterministic_metadata.keys():
+            key_in_file = fr"/{metadata_key}" in file
+            if success_expected:  # metadata keys should not be in the file
+                assert not key_in_file
+            else:
+                assert key_in_file
+
+    else:  # "eps" or "png"
+        actual_metadata = Image.open(str(baseline_image)).info
+        for k, expected in deterministic_metadata.items():
+            actual = actual_metadata.get(k, None)
+            if success_expected:  # metadata keys should not be in the file
+                if expected is None:
+                    assert actual is None
+                else:
+                    assert actual == expected
+            else:  # metadata keys should still be in the file
+                if expected is None:
+                    assert actual is not None
+                else:
+                    assert actual != expected

tests/test_hash_library.py

@@ -24,6 +24,7 @@ def test_config(pytester, ini, cli, kwarg, success_expected):
     pytester.makeini(
         f"""
         [pytest]
+        mpl-deterministic: true
         {ini}
         """
     )

tests/test_pytest_mpl.py

@@ -9,7 +9,7 @@
 import matplotlib.ft2font
 import matplotlib.pyplot as plt
 import pytest
-from matplotlib.testing.compare import converter
+from helpers import skip_if_format_unsupported
 from packaging.version import Version
 
 MPL_VERSION = Version(matplotlib.__version__)
@@ -668,31 +668,14 @@ def test_raises():
 @pytest.mark.parametrize('use_hash_library', (False, True))
 @pytest.mark.parametrize('passes', (False, True))
 @pytest.mark.parametrize("file_format", ['eps', 'pdf', 'png', 'svg'])
-@pytest.mark.skipif(not hash_library.exists(), reason="No hash library for this mpl version")
 def test_formats(pytester, use_hash_library, passes, file_format):
     """
     Note that we don't test all possible formats as some do not compress well
     and would bloat the baseline directory.
     """
-
-    if file_format == 'svg' and MPL_VERSION < Version('3.3'):
-        pytest.skip('SVG comparison is only supported in Matplotlib 3.3 and above')
-
-    if use_hash_library:
-
-        if file_format == 'pdf' and MPL_VERSION < Version('2.1'):
-            pytest.skip('PDF hashes are only deterministic in Matplotlib 2.1 and above')
-        elif file_format == 'eps' and MPL_VERSION < Version('2.1'):
-            pytest.skip('EPS hashes are only deterministic in Matplotlib 2.1 and above')
-
-    if use_hash_library and not sys.platform.startswith('linux'):
-        pytest.skip('Hashes for vector graphics are only provided in the hash library for Linux')
-
-    if file_format != 'png' and file_format not in converter:
-        if file_format == 'svg':
-            pytest.skip('Comparing SVG files requires inkscape to be installed')
-        else:
-            pytest.skip('Comparing EPS and PDF files requires ghostscript to be installed')
+    skip_if_format_unsupported(file_format, using_hashes=use_hash_library)
+    if use_hash_library and not hash_library.exists():
+        pytest.skip("No hash library for this mpl version")
 
     pytester.makepyfile(
         f"""