From 544245429ca7abdbc57dd1c9e7cb1d27e6ce175e Mon Sep 17 00:00:00 2001
From: Zebedee Nicholls <zebedee.nicholls@climate-energy-college.org>
Date: Fri, 19 Jan 2024 13:50:35 +0100
Subject: [PATCH] Fix up tests

---
 README.md                                |  4 +--
 tests/conftest.py                        | 14 ++++++++++
 tests/integration/test_ciceroSCM.py      | 33 ++++++++++++------------
 tests/integration/test_magicc7.py        | 22 ++++++++--------
 tests/integration/test_run_multimodel.py |  6 ++---
 5 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/README.md b/README.md
index 2daec4fa..492c64a6 100644
--- a/README.md
+++ b/README.md
@@ -2,9 +2,10 @@
 
 TODO:
 
+- Change to using pytest-regressions for handling expected values
+  - next PR as too hard to do with confidence when there are so many moving parts
 - Update notebook tests i.e. just make sure docs building covers them
 - TODO: work out how to handle package data
-- Change to using pytest-regressions for handling expected values
 - get version to include dirty/commits when being created
 - fix deprecated API warning in scmdata for both seaborn and pandas (seaborn b/c of "ci" usage, pandas because of something in concat, run the FaIR notebook to see them both)
 - add PHONY to help in copier
@@ -25,7 +26,6 @@ TODO:
   - only push to your own branches (semi-enforced by GitHub's forking workflow but a good reminder)
   - tools we use
   - small MRs
-- other learnings from the basic copier setup into copier
 
 <!---
 Can use start-after and end-before directives in docs, see
diff --git a/tests/conftest.py b/tests/conftest.py
index 449ca1b0..deae0c48 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -106,3 +106,17 @@ def test_scenario_ssp370_world(test_data_dir: Path) -> scmdata.ScmRun:
     )
 
     return scenario
+
+
+def pytest_addoption(parser):
+    parser.addoption(
+        "--update-expected-values",
+        action="store_true",
+        default=False,
+        help="Overwrite expected values",
+    )
+
+
+@pytest.fixture
+def update_expected_values(request):
+    return request.config.getoption("--update-expected-values")
diff --git a/tests/integration/test_ciceroSCM.py b/tests/integration/test_ciceroSCM.py
index 5af70d60..31f6b0dc 100644
--- a/tests/integration/test_ciceroSCM.py
+++ b/tests/integration/test_ciceroSCM.py
@@ -5,7 +5,7 @@
 import pytest
 from scmdata import ScmRun
 
-from openscm_runner import run
+import openscm_runner.run
 from openscm_runner.adapters import CICEROSCM
 from openscm_runner.adapters.ciceroscm_adapter import write_parameter_files
 from openscm_runner.testing import _AdapterTester
@@ -36,7 +36,7 @@ def test_run(
             tmp = tmp[cols[1:] + cols[:1]]
             test_scenarios = ScmRun(test_scenarios)
 
-        res = run(
+        res = openscm_runner.run.run(
             scenarios=test_scenarios.filter(scenario=["ssp126", "ssp245", "ssp370"]),
             climate_models_cfgs={
                 "CICEROSCM": [
@@ -151,7 +151,7 @@ def test_run(
         )
 
         # check that emissions were passed through correctly
-        for (scen, variable, unit, exp_val) in (
+        for scen, variable, unit, exp_val in (
             ("ssp126", "Emissions|CO2", "PgC/yr", -2.3503),
             ("ssp370", "Emissions|CO2", "PgC/yr", 22.562),
             ("ssp126", "Emissions|CH4", "TgCH4/yr", 122.195),
@@ -163,14 +163,14 @@ def test_run(
                 variable=variable, year=2100, scenario=scen
             ).convert_unit(unit)
             if res_scen_2100_emms.empty:
-                raise AssertionError("No {} data for {}".format(variable, scen))
+                raise AssertionError(f"No {variable} data for {scen}")  # noqa: TRY003
 
             npt.assert_allclose(
                 res_scen_2100_emms.values,
                 exp_val,
                 rtol=1e-4,
             )
-        for (scen, variable, unit, exp_val14, exp_val16) in (
+        for scen, variable, unit, exp_val14, exp_val16 in (
             ("ssp126", "Emissions|CH4", "TgCH4/yr", 387.874, 379.956),
             ("ssp370", "Emissions|CH4", "TgCH4/yr", 387.874, 394.149),
             ("ssp126", "Emissions|N2O", "TgN2ON/yr", 6.911, 6.858),
@@ -180,13 +180,13 @@ def test_run(
                 variable=variable, year=2014, scenario=scen
             ).convert_unit(unit)
             if res_scen_2014_emms.empty:
-                raise AssertionError("No {} data for {}".format(variable, scen))
+                raise AssertionError(f"No {variable} data for {scen}")  # noqa: TRY003
 
             res_scen_2016_emms = res.filter(
                 variable=variable, year=2016, scenario=scen
             ).convert_unit(unit)
             if res_scen_2016_emms.empty:
-                raise AssertionError("No {} data for {}".format(variable, scen))
+                raise AssertionError(f"No {variable} data for {scen}")  # noqa: TRY003
 
             npt.assert_allclose(
                 res_scen_2014_emms.values,
@@ -198,13 +198,13 @@ def test_run(
                 exp_val16,
                 rtol=1e-4,
             )
-        for (scen, variable) in (
+        for scen, variable in (
             ("ssp126", "Effective Radiative Forcing|Aerosols"),
             ("ssp370", "Effective Radiative Forcing|Aerosols"),
         ):
             res_scen_2015_emms = res.filter(variable=variable, year=2015, scenario=scen)
             if res_scen_2015_emms.empty:
-                raise AssertionError("No CO2 emissions data for {}".format(scen))
+                raise AssertionError(f"No CO2 emissions data for {scen}")  # noqa: TRY003
 
             assert not np.equal(res_scen_2015_emms.values, 0).all()
 
@@ -255,11 +255,11 @@ def test_run(
     def test_variable_naming(self, test_scenarios):
         missing_from_ciceroscm = (
             "Effective Radiative Forcing|Aerosols|Direct Effect|BC|MAGICC AFOLU",
-            "Effective Radiative Forcing|Aerosols|Direct Effect|BC|MAGICC Fossil and Industrial",
+            "Effective Radiative Forcing|Aerosols|Direct Effect|BC|MAGICC Fossil and Industrial",  # noqa: E501
             "Effective Radiative Forcing|Aerosols|Direct Effect|OC|MAGICC AFOLU",
-            "Effective Radiative Forcing|Aerosols|Direct Effect|OC|MAGICC Fossil and Industrial",
+            "Effective Radiative Forcing|Aerosols|Direct Effect|OC|MAGICC Fossil and Industrial",  # noqa: E501
             "Effective Radiative Forcing|Aerosols|Direct Effect|SOx|MAGICC AFOLU",
-            "Effective Radiative Forcing|Aerosols|Direct Effect|SOx|MAGICC Fossil and Industrial",
+            "Effective Radiative Forcing|Aerosols|Direct Effect|SOx|MAGICC Fossil and Industrial",  # noqa: E501
             "Heat Uptake|Ocean",
             "Net Atmosphere to Land Flux|CO2",
             "Net Atmosphere to Ocean Flux|CO2",
@@ -267,7 +267,7 @@ def test_variable_naming(self, test_scenarios):
         common_variables = [
             c for c in self._common_variables if c not in missing_from_ciceroscm
         ]
-        res = run(
+        res = openscm_runner.run.run(
             climate_models_cfgs={"CICEROSCM": ({"lambda": 0.540},)},
             scenarios=test_scenarios.filter(scenario="ssp126"),
             output_variables=common_variables,
@@ -280,7 +280,7 @@ def test_variable_naming(self, test_scenarios):
     @pytest.mark.ciceroscm
     def test_w_out_config(self, test_scenarios):
         with pytest.raises(NotImplementedError):
-            run(
+            openscm_runner.run.run(
                 scenarios=test_scenarios.filter(scenario=["ssp126"]),
                 climate_models_cfgs={
                     "CiceroSCM": [
@@ -320,7 +320,8 @@ def test_write_parameter_files(self, input, exp):
     @pytest.mark.parametrize(
         "name",
         (
-            "some super super super super super super super super super super long scenario name which should not explode",
+            "some super super super super super super super super "
+            "super super long scenario name which should not explode",
             "another name with some special | . characters which should also work",
             # any other names you want to test
         ),
@@ -334,7 +335,7 @@ def test_run_long_scenario_name(
             {"scenario": {"ssp126": name}}
         )
 
-        res = run(
+        res = openscm_runner.run.run(
             scenarios=starting_scenario,
             climate_models_cfgs={"CICEROSCM": [{}]},
             output_variables=("Surface Air Temperature Change",),
diff --git a/tests/integration/test_magicc7.py b/tests/integration/test_magicc7.py
index 5cdfabda..8353bd69 100644
--- a/tests/integration/test_magicc7.py
+++ b/tests/integration/test_magicc7.py
@@ -4,7 +4,7 @@
 import pytest
 from scmdata import ScmRun
 
-from openscm_runner import run
+import openscm_runner.run
 from openscm_runner.adapters import MAGICC7
 from openscm_runner.testing import _AdapterTester
 from openscm_runner.utils import calculate_quantiles
@@ -24,7 +24,7 @@ def test_run(
             "expected_magicc7_test_run_output.json",
         )
 
-        res = run(
+        res = openscm_runner.run.run(
             climate_models_cfgs={
                 "MAGICC7": [
                     {
@@ -75,9 +75,10 @@ def test_run(
         assert isinstance(res, ScmRun)
         assert res["run_id"].min() == 0
         assert res["run_id"].max() == 8
-        assert res.get_unique_meta(
-            "climate_model", no_duplicates=True
-        ) == "MAGICC{}".format(MAGICC7.get_version())
+        assert (
+            res.get_unique_meta("climate_model", no_duplicates=True)
+            == f"MAGICC{MAGICC7.get_version()}"
+        )
         assert set(res.get_unique_meta("variable")) == {
             "Surface Air Temperature Change",
             "Effective Radiative Forcing",
@@ -102,7 +103,7 @@ def test_run(
 
     def test_variable_naming(self, test_scenarios):
         common_variables = self._common_variables
-        res = run(
+        res = openscm_runner.run.run(
             climate_models_cfgs={"MAGICC7": ({"core_climatesensitivity": 3},)},
             scenarios=test_scenarios.filter(scenario="ssp126"),
             output_variables=common_variables,
@@ -131,8 +132,7 @@ def test_write_scen_files_and_make_full_cfgs(test_scenarios):
         ["model", "scenario"]
     ):
         scen_file_name = (
-            "{}_{}.SCEN7".format(scenario, model)
-            .upper()
+            f"{scenario}_{model}.SCEN7".upper()
             .replace("/", "-")
             .replace("\\", "-")
             .replace(" ", "-")
@@ -149,7 +149,7 @@ def test_write_scen_files_and_make_full_cfgs(test_scenarios):
         assert scenario_cfg["model"] == model
         assert scenario_cfg["scenario"] == scenario
         for i in range(2, 9):
-            scen_flag_val = scenario_cfg["file_emisscen_{}".format(i)]
+            scen_flag_val = scenario_cfg[f"file_emisscen_{i}"]
 
             assert scen_flag_val == "NONE"
 
@@ -181,7 +181,7 @@ def test_return_config(test_scenarios, out_config):
                 }
             )
 
-    res = run(
+    res = openscm_runner.run.run(
         climate_models_cfgs={"MAGICC7": cfgs},
         scenarios=test_scenarios.filter(scenario=["ssp126", "ssp245", "ssp370"]),
         output_variables=(
@@ -217,7 +217,7 @@ def test_return_config(test_scenarios, out_config):
 )
 def test_return_config_clash_error(test_scenarios, cfgs):
     with pytest.raises(ValueError):
-        run(
+        openscm_runner.run.run(
             climate_models_cfgs={"MAGICC7": cfgs},
             scenarios=test_scenarios.filter(scenario=["ssp126"]),
             output_variables=("Surface Air Temperature Change",),
diff --git a/tests/integration/test_run_multimodel.py b/tests/integration/test_run_multimodel.py
index db5453c1..d244b230 100644
--- a/tests/integration/test_run_multimodel.py
+++ b/tests/integration/test_run_multimodel.py
@@ -5,8 +5,8 @@
 import pytest
 from scmdata import ScmRun
 
+import openscm_runner.run
 import openscm_runner.testing
-from openscm_runner import run
 from openscm_runner.utils import calculate_quantiles
 
 RTOL = 1e-5
@@ -19,7 +19,7 @@ def _check_res(exp, check_val, raise_error, rtol=RTOL):
         if raise_error:
             raise
 
-        print("exp: {}, check_val: {}".format(exp, check_val))
+        print(f"exp: {exp}, check_val: {check_val}")
 
 
 @pytest.mark.magicc
@@ -30,7 +30,7 @@ def test_multimodel_run(test_scenarios, test_data_dir, update_expected_values):
         "expected_run_multimodel_output.json",
     )
 
-    res = run(
+    res = openscm_runner.run.run(
         climate_models_cfgs={
             "FaIR": [
                 {},