From 5c189be50433d10014e50dcd59da8281042cb9ac Mon Sep 17 00:00:00 2001 From: John Sharples <41682323+John-Sharples@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:46:01 -0600 Subject: [PATCH] Feature 461 make compare images configurable (#467) * 461: scatter tests and json compare * 461: tidy up docs and whitespace * 461: remove accidental git add * 461: Make CompareImages configureable by env var * 461: fix spurious fails * 461: fix base dir setting * 461: add omit to coverage report * 461: remove redundant .coveragerc --- .coveragerc | 2 - pyproject.toml | 5 +++ test/bar/test_bar.py | 5 +-- test/box/test_box.py | 3 +- test/conftest.py | 33 +++++++++++++-- test/contour/test_contour.py | 4 +- test/eclv/test_eclv.py | 3 +- test/ens_ss/test_ens_ss.py | 4 +- .../test_equivalence_testing_bounds.py | 3 +- test/histogram/test_prob_hist.py | 4 +- test/histogram/test_rank_hist.py | 3 +- test/histogram/test_rel_hist.py | 3 +- test/histogram_2d/test_histogram_2d.py | 2 - test/hovmoeller/test_hovmoeller.py | 5 +-- test/line/test_line_groups_plot.py | 4 +- test/line/test_line_plot.py | 14 ++----- test/mpr_plot/test_mpr_plot.py | 3 +- .../test_performance_diagram.py | 3 +- .../test_reliability_diagram.py | 3 +- test/revision_box/test_revision_box.py | 3 +- test/revision_series/test_revision_series.py | 4 +- test/roc_diagram/test_roc_diagram.py | 3 +- test/scatter/test_scatter.py | 1 - test/taylor_diagram/test_taylor_diagram.py | 41 +++++++++---------- test/wind_rose/test_wind_rose.py | 3 +- 25 files changed, 82 insertions(+), 79 deletions(-) delete mode 100644 .coveragerc diff --git a/.coveragerc b/.coveragerc deleted file mode 100644 index 4edd7b1a..00000000 --- a/.coveragerc +++ /dev/null @@ -1,2 +0,0 @@ -[run] -relative_files = True diff --git a/pyproject.toml b/pyproject.toml index d5e28575..f8bf649d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,6 +33,11 @@ testpaths = ["test"] [tool.coverage.run] source = ["metplotpy/plots"] +omit = [ + "config.py", + "config-3.py", + ] +relative_files = true [tool.coverage.report] exclude_also = [ diff --git a/test/bar/test_bar.py b/test/bar/test_bar.py index eb0c5fee..4cd474c9 100644 --- a/test/bar/test_bar.py +++ b/test/bar/test_bar.py @@ -3,7 +3,7 @@ import pytest from metplotpy.plots.bar import bar -# from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['bar.png', 'bar.points1'] @@ -71,7 +71,6 @@ def test_no_nans_in_points_file(setup, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("fails on linux host machines") def test_images_match(setup, remove_files): """ Compare an expected plot with the @@ -83,7 +82,6 @@ def test_images_match(setup, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("fails on linux host machines") def test_none_data_images_match(setup_nones): """ Compare an expected plot with the @@ -141,7 +139,6 @@ def test_point_and_plot_files_exist_default(setup_env, remove_files): remove_files(cwd, check_files) -@pytest.mark.skip("fails on linux host machines") def test_threshold_plotting(setup_env, remove_files): """ Verify that the bar plot using data with thresholds is correct. diff --git a/test/box/test_box.py b/test/box/test_box.py index 25ab21c4..90f1eec8 100644 --- a/test/box/test_box.py +++ b/test/box/test_box.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.box import box -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['box.png', 'box.points1'] @@ -28,7 +28,6 @@ def test_files_exist(setup, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("fails on linux hosts") def test_images_match(setup): """ Compare an expected plot with the diff --git a/test/conftest.py b/test/conftest.py index 2555b4cc..264258c2 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,5 +1,6 @@ import pytest import os +from unittest.mock import patch import shutil import json import xarray as xr @@ -10,13 +11,38 @@ # realative file locations can be used for each test # file. # NOTE: autouse=True means this applies to ALL tests. -# Code that updates the cwd inside a test file is now -# redundant and can be deleted. +# Code that updates the cwd inside test is now redundant +# and can be deleted. @pytest.fixture(autouse=True) def change_test_dir(request, monkeypatch): monkeypatch.chdir(request.fspath.dirname) +@pytest.fixture(autouse=True) +def patch_CompareImages(request): + """This fixture controls the use of CompareImages in the + test suite. By default, all calls to CompareImages will + result in the test skipping. To change this behaviour set + an env var METPLOTPY_COMPAREIMAGES + """ + if bool(os.getenv("METPLOTPY_COMPAREIMAGES")): + yield + else: + class mock_CompareImages: + def __init__(self, img1, img2): + # TODO: rather than skip we could inject an alternative + # comparison that is more relaxed. To do this, extend + # this this class to generate a self.mssim value. + pytest.skip("CompareImages not enabled in pytest. " + "To enable `export METPLOTPY_COMPAREIMAGES=$true`") + try: + with patch.object(request.module, 'CompareImages', mock_CompareImages) as mock_ci: + yield mock_ci + except AttributeError: + # test module doesn't import CompareImages. Do nothing. + yield + + def ordered(obj): """Recursive function to sort JSON, even lists of dicts with the same keys""" if isinstance(obj, dict): @@ -26,7 +52,6 @@ def ordered(obj): else: return obj - @pytest.fixture def assert_json_equal(): def compare_json(fig, expected_json_file): @@ -36,7 +61,6 @@ def compare_json(fig, expected_json_file): actual = json.loads(fig.to_json(), parse_float=str, parse_int=str) with open(expected_json_file) as f: expected = json.load(f,parse_float=str, parse_int=str) - # Fail with a nice message if ordered(actual) == ordered(expected): return True @@ -79,6 +103,7 @@ def remove_the_files(test_dir, file_list): return remove_the_files +# data for netCDF file TEST_NC_DATA = xr.Dataset( { "precip": xr.DataArray( diff --git a/test/contour/test_contour.py b/test/contour/test_contour.py index 7aff0c56..a7208b9e 100644 --- a/test/contour/test_contour.py +++ b/test/contour/test_contour.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.contour import contour -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -42,7 +42,7 @@ def test_files_exist(setup, test_input, expected): assert os.path.isfile(test_input) == expected cleanup() -@pytest.mark.skip("fails on linux hosts") + def test_images_match(setup): """ Compare an expected plots with the diff --git a/test/eclv/test_eclv.py b/test/eclv/test_eclv.py index 37bf24b6..82144cbc 100644 --- a/test/eclv/test_eclv.py +++ b/test/eclv/test_eclv.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.eclv import eclv -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -49,7 +49,6 @@ def test_files_exist(setup, test_input, expected): assert os.path.isfile(test_input) == expected cleanup() -@pytest.mark.skip("fails on linux hosts") def test_images_match(setup): """ Compare an expected plots with the diff --git a/test/ens_ss/test_ens_ss.py b/test/ens_ss/test_ens_ss.py index 9962fc56..9117038b 100644 --- a/test/ens_ss/test_ens_ss.py +++ b/test/ens_ss/test_ens_ss.py @@ -1,6 +1,6 @@ import os import pytest -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages from metplotpy.plots.ens_ss import ens_ss cwd = os.path.dirname(__file__) @@ -43,7 +43,7 @@ def test_files_exist( setup, test_input, expected): assert os.path.isfile(test_input) == expected cleanup() -@pytest.mark.skip("fails on linux hosts") + def test_images_match(setup): """ Compare an expected plot with the diff --git a/test/equivalence_testing_bounds/test_equivalence_testing_bounds.py b/test/equivalence_testing_bounds/test_equivalence_testing_bounds.py index e7e849e6..4235dda2 100644 --- a/test/equivalence_testing_bounds/test_equivalence_testing_bounds.py +++ b/test/equivalence_testing_bounds/test_equivalence_testing_bounds.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.equivalence_testing_bounds import equivalence_testing_bounds as etb -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['equivalence_testing_bounds.png', 'equivalence_testing_bounds.points1'] @@ -29,7 +29,6 @@ def test_files_exist(setup, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("skimage differences causing failure") def test_images_match(setup, remove_files): ''' Compare an expected plot with the diff --git a/test/histogram/test_prob_hist.py b/test/histogram/test_prob_hist.py index fc9b3ab4..7e830b3b 100644 --- a/test/histogram/test_prob_hist.py +++ b/test/histogram/test_prob_hist.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.histogram import prob_hist -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -35,7 +35,7 @@ def test_files_exist(setup, test_input, expected): assert os.path.isfile(f"{cwd}/{test_input}") == expected cleanup() -@pytest.mark.skip("Image comparisons fail during Github Actions checks.") + def test_images_match(setup): """ Compare an expected plot with the diff --git a/test/histogram/test_rank_hist.py b/test/histogram/test_rank_hist.py index 4b9d5b08..17fe34ee 100644 --- a/test/histogram/test_rank_hist.py +++ b/test/histogram/test_rank_hist.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.histogram import rank_hist -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -36,7 +36,6 @@ def test_files_exist(setup, test_input, expected): cleanup() -@pytest.mark.skip("Image comparisons fail during Github Actions checks.") def test_images_match(setup): """ Compare an expected plot with the diff --git a/test/histogram/test_rel_hist.py b/test/histogram/test_rel_hist.py index a491283c..37f98ccd 100644 --- a/test/histogram/test_rel_hist.py +++ b/test/histogram/test_rel_hist.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.histogram import rel_hist -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -39,7 +39,6 @@ def test_files_exist(setup, test_input, expected): cleanup() -@pytest.mark.skip("Image comparisons fail in Github Actions checks.") def test_images_match(setup): """ Compare an expected plot with the diff --git a/test/histogram_2d/test_histogram_2d.py b/test/histogram_2d/test_histogram_2d.py index 1bc96a04..e8363a96 100644 --- a/test/histogram_2d/test_histogram_2d.py +++ b/test/histogram_2d/test_histogram_2d.py @@ -3,8 +3,6 @@ from metplotpy.plots.histogram_2d import histogram_2d as h2d -# from metcalcpy.compare_images import CompareImages - cwd = os.path.dirname(__file__) diff --git a/test/hovmoeller/test_hovmoeller.py b/test/hovmoeller/test_hovmoeller.py index 9fae2d40..7bb9a158 100644 --- a/test/hovmoeller/test_hovmoeller.py +++ b/test/hovmoeller/test_hovmoeller.py @@ -2,7 +2,7 @@ import pytest import metplotpy.plots.hovmoeller.hovmoeller as hov from metplotpy.plots import util -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages def dict_to_yaml(data_dict, output_yaml = "test_hovmoeller.yaml"): @@ -22,8 +22,7 @@ def cleanup(file_to_remove): # don't exist. Ignore. pass - -@pytest.mark.skip() +@pytest.mark.skip("Requires specific netCDF input") def test_default_plot_images_match(): ''' Compare an expected plot with the diff --git a/test/line/test_line_groups_plot.py b/test/line/test_line_groups_plot.py index a0756c03..21fd4b26 100644 --- a/test/line/test_line_groups_plot.py +++ b/test/line/test_line_groups_plot.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.line import line as l -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -47,7 +47,7 @@ def test_files_exist(setup, test_input, expected): assert os.path.isfile(test_input) == expected cleanup() -@pytest.mark.skip("fails on linux hosts") + def test_images_match(setup): ''' Compare an expected plot with the diff --git a/test/line/test_line_plot.py b/test/line/test_line_plot.py index bb4972dc..4778ce03 100644 --- a/test/line/test_line_plot.py +++ b/test/line/test_line_plot.py @@ -5,7 +5,7 @@ cwd = os.path.dirname(__file__) -# from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages @pytest.fixture @@ -147,7 +147,6 @@ def test_no_nans_in_points_files(): pass -@pytest.mark.skip() def test_images_match(setup): ''' Compare an expected plot with the @@ -164,7 +163,6 @@ def test_images_match(setup): cleanup() -@pytest.mark.skip() def test_new_images_match(): ''' Compare an expected plot with the start_at_zero option, with the @@ -323,21 +321,20 @@ def test_fixed_var_val(): pass -@pytest.mark.skip("Image comparison for development only due to differences in hosts") +@pytest.mark.skip("This test currently raises an error on line 81 of line.py. This can" + " probably be fixed be revisiting fbias_fixed_vars_vals.yaml") def test_fixed_var_val_image_compare(): """ Verify that the fixed_vars_vals_input setting reproduces the expected plot. """ - from metcalcpy.compare_images import CompareImages # Set up the METPLOTPY_BASE so that met_plot.py will correctly find # the config directory containing all the default config files. os.environ['METPLOTPY_BASE'] = f"{cwd}/../../" os.environ['TEST_DIR'] = cwd custom_config_filename = f"{cwd}/fbias_fixed_vars_vals.yaml" - # Invoke the command to generate a line plot based on # the custom config file. l.main(custom_config_filename) @@ -349,10 +346,7 @@ def test_fixed_var_val_image_compare(): created_file = os.path.join(cwd, plot_file) # first verify that the output plot was created - if os.path.exists(created_file): - assert True - else: - assert False + assert os.path.exists(created_file) comparison = CompareImages(expected_plot, created_file) diff --git a/test/mpr_plot/test_mpr_plot.py b/test/mpr_plot/test_mpr_plot.py index ee724505..0dc2d237 100644 --- a/test/mpr_plot/test_mpr_plot.py +++ b/test/mpr_plot/test_mpr_plot.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.mpr_plot import mpr_plot -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['mpr_plots.png'] @@ -29,7 +29,6 @@ def test_files_exist(setup, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("unreliable-sometimes fails due to differences between machines.") def test_images_match(setup, remove_files): """ Compare an expected plot with the diff --git a/test/performance_diagram/test_performance_diagram.py b/test/performance_diagram/test_performance_diagram.py index 511e8a8d..c4ecfbbc 100644 --- a/test/performance_diagram/test_performance_diagram.py +++ b/test/performance_diagram/test_performance_diagram.py @@ -2,7 +2,7 @@ import pytest from metplotpy.plots.performance_diagram import performance_diagram as pd -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -89,7 +89,6 @@ def test_files_exist(setup_env, test_input, expected_bool, remove_files): pass -@pytest.mark.skip() def test_images_match(setup, remove_files): ''' Compare an expected plot with the diff --git a/test/reliability_diagram/test_reliability_diagram.py b/test/reliability_diagram/test_reliability_diagram.py index 0ceeb222..fe97773c 100644 --- a/test/reliability_diagram/test_reliability_diagram.py +++ b/test/reliability_diagram/test_reliability_diagram.py @@ -2,7 +2,7 @@ import pytest import os from metplotpy.plots.reliability_diagram import reliability as r -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['reliability.png', 'reliability.points1'] @@ -30,7 +30,6 @@ def test_files_exist(setup, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("depends on machine on which this is run") def test_images_match(setup, remove_files): ''' Compare an expected plot with the diff --git a/test/revision_box/test_revision_box.py b/test/revision_box/test_revision_box.py index 6932327f..afaa43f0 100644 --- a/test/revision_box/test_revision_box.py +++ b/test/revision_box/test_revision_box.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.revision_box import revision_box -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['revision_box.png', 'revision_box.points1'] @@ -29,7 +29,6 @@ def test_files_exist(setup, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("fails on linux hosts") def test_images_match(setup, remove_files): """ Compare an expected plot with the diff --git a/test/revision_series/test_revision_series.py b/test/revision_series/test_revision_series.py index c381900a..1c504eb7 100644 --- a/test/revision_series/test_revision_series.py +++ b/test/revision_series/test_revision_series.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.revision_series import revision_series -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ('revision_series.png', 'revision_series.points1') @@ -29,7 +29,7 @@ def test_files_exist(setup, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("fails on linux hosts") +# @pytest.mark.skip("fails on linux hosts") def test_images_match(setup, remove_files): """ Compare an expected plot with the diff --git a/test/roc_diagram/test_roc_diagram.py b/test/roc_diagram/test_roc_diagram.py index a5c22d37..c22d2d70 100644 --- a/test/roc_diagram/test_roc_diagram.py +++ b/test/roc_diagram/test_roc_diagram.py @@ -4,7 +4,7 @@ import os import pandas as pd from metplotpy.plots.roc_diagram import roc_diagram as roc -# from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages import metcalcpy.util.ctc_statistics as ctc cwd = os.path.dirname(__file__) @@ -209,7 +209,6 @@ def test_ee_returns_empty_df(capsys, remove_files): remove_files(cwd, ['CTC_ROC_ee.png', 'CTC_ROC_ee.html']) -@pytest.mark.skip("skip image comparison") def test_images_match(setup, remove_files): ''' Compare an expected plot with the diff --git a/test/scatter/test_scatter.py b/test/scatter/test_scatter.py index 77697780..ef2f91bf 100644 --- a/test/scatter/test_scatter.py +++ b/test/scatter/test_scatter.py @@ -10,4 +10,3 @@ def test_scatter(assert_json_equal): def test_main(): # check that main can execute without error. scatter.main() - \ No newline at end of file diff --git a/test/taylor_diagram/test_taylor_diagram.py b/test/taylor_diagram/test_taylor_diagram.py index 415bee98..d1fe6573 100644 --- a/test/taylor_diagram/test_taylor_diagram.py +++ b/test/taylor_diagram/test_taylor_diagram.py @@ -1,6 +1,6 @@ import os from metplotpy.plots.taylor_diagram import taylor_diagram as td -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) @@ -36,24 +36,23 @@ def test_pos_corr_file_exists(setup_env): # Clean up os.remove(plot_file) -# Not reliable when the expected image is generated on a Mac and then this -# test is run on non-Mac machine. -# def test_pos_corr_images_match(): -# os.environ['METPLOTPY_BASE'] = "../../metplotpy" -# test_config_filename = "test_pos_corr.yaml" -# td.main(test_config_filename) -# -# # Verify that a plot was generated -# plot_file = "test_pos_corr_plot.png" -# expected_file = "expected_pos_corr_plot.png" -# path = os.getcwd() -# -# # image comparison -# comparison = CompareImages(plot_file, expected_file) -# assert comparison.mssim >= .99 -# -# # Clean up -# os.remove(os.path.join(path, plot_file)) + +def test_pos_corr_images_match(): + os.environ['METPLOTPY_BASE'] = f"{cwd}/../../" + test_config_filename = "test_pos_corr.yaml" + td.main(test_config_filename) + + # Verify that a plot was generated + plot_file = "test_pos_corr_plot.png" + expected_file = "expected_pos_corr_plot.png" + path = os.getcwd() + + # image comparison + comparison = CompareImages(plot_file, expected_file) + assert comparison.mssim >= .99 + + # Clean up + os.remove(os.path.join(path, plot_file)) def test_neg_and_pos_corr_file_exists(setup_env): @@ -81,8 +80,8 @@ def test_neg_and_pos_corr_images_match(setup_env): expected_file = f"{cwd}/expected_neg_and_pos_corr_plot.png" # image comparison, with allowance of .99 match instead of 100% match - #comparison = CompareImages(plot_file, expected_file) - #assert comparison.mssim >= .99 + comparison = CompareImages(plot_file, expected_file) + assert comparison.mssim >= .99 # Clean up os.remove(plot_file) diff --git a/test/wind_rose/test_wind_rose.py b/test/wind_rose/test_wind_rose.py index 04ac6284..1465b766 100644 --- a/test/wind_rose/test_wind_rose.py +++ b/test/wind_rose/test_wind_rose.py @@ -1,7 +1,7 @@ import pytest import os from metplotpy.plots.wind_rose import wind_rose -#from metcalcpy.compare_images import CompareImages +from metcalcpy.compare_images import CompareImages cwd = os.path.dirname(__file__) CLEANUP_FILES = ['wind_rose_custom.png', 'point_stat_mpr.points1'] @@ -90,7 +90,6 @@ def test_points1_files_exist(setup_env, test_input, expected, remove_files): remove_files(cwd, CLEANUP_FILES) -@pytest.mark.skip("unreliable sometimes fails due to differences in machines.") def test_images_match(setup, remove_files): ''' Compare an expected plot with the