Suggested changes

CHANGELOG.rst

@@ -4,7 +4,7 @@ Changelog
 master
 ------
 
-- (`#252 <https://github.com/openclimatedata/pymagicc/pull/256>`_) Capture stderr output from magicc
+- (`#252 <https://github.com/openclimatedata/pymagicc/pull/256>`_) Capture stderr output from MAGICC7 and above (not available in MAGICC6)
 - (`#252 <https://github.com/openclimatedata/pymagicc/pull/252>`_) Improve header writing, upgrade MAGICC time conversions and fix wine not installed error handling
 - (`#253 <https://github.com/openclimatedata/pymagicc/pull/253>`_) Add support for ``out_dynamic_vars`` parameter
 - (`#250 <https://github.com/openclimatedata/pymagicc/pull/250>`_) Add support for ``.MAG`` files

pymagicc/core.py

@@ -241,16 +241,16 @@ def _deep_update(b, o):
 
         return self._default_config
 
-    def run(self, scenario=None, only=None, debug=False, verbose=False, **kwargs):
+    def run(self, scenario=None, only=None, debug=False, **kwargs):
         """
         Run MAGICC and parse the output.
 
         As a reminder, putting ``out_parameters=1`` will cause MAGICC to write out its
         parameters into ``out/PARAMETERS.OUT`` and they will then be read into
         ``output.metadata["parameters"]`` where ``output`` is the returned object.
 
-        Any logged output from running magicc will be in``output.metadata["stderr"]``. The amount of logs can be controlled with
-        the ``debug`` and ``verbose`` parameters.
+        Any logged output from running magicc will be in``output.metadata["stderr"]``.
+        The level of logging can be controlled with the ``debug`` argument.
 
         Parameters
         ----------
@@ -261,11 +261,9 @@ def run(self, scenario=None, only=None, debug=False, verbose=False, **kwargs):
         only : list of str
             If not None, only extract variables in this list.
 
-        debug: bool
-            If true, then magicc will run in debug mode with the maximum amount of logging.
-
-        verbose: bool
-            If True, magicc will run be verbose mode.
+        debug: {True, False, "verbose"}
+            If true, MAGICC will run in debug mode with the maximum amount of logging.
+            If "verbose", MAGICC will be run in verbose mode.
 
         kwargs
             Other config values to pass to MAGICC for the run
@@ -281,8 +279,10 @@ def run(self, scenario=None, only=None, debug=False, verbose=False, **kwargs):
         ------
         ValueError
             If no output is found which matches the list specified in ``only``.
+
         subprocess.CalledProcessError
-            If magicc fails to run. Check the stderr property on the exception to inspect the results output from magicc
+            If MAGICC fails to run. Check the 'stderr' key of the result's `metadata`
+            attribute to inspect the results output from MAGICC.
         """
         if not exists(self.root_dir):
             raise FileNotFoundError(self.root_dir)
@@ -316,10 +316,12 @@ def run(self, scenario=None, only=None, debug=False, verbose=False, **kwargs):
         exec_dir = basename(self.original_dir)
         command = [join(self.root_dir, exec_dir, self.binary_name)]
         if self.version >= 7:
-            if debug:
-                command.append("--debug")
-            elif verbose:
+            if debug == "verbose":
                 command.append("--verbose")
+            elif debug:
+                command.append("--debug")
+        elif debug:
+            raise ValueError("MAGICC6 has no debug capability")
 
         if not IS_WINDOWS and self.binary_name.endswith(".exe"):  # pragma: no cover
             if not _wine_installed:
@@ -377,7 +379,7 @@ def run(self, scenario=None, only=None, debug=False, verbose=False, **kwargs):
         for l in levels_to_warn:
             if l in mdata.metadata["stderr"]:
                 warnings.warn(
-                    'magicc logged an {} message. Check the metadata["stderr"] attribute'.format(
+                    "magicc logged a {} message. Check the 'stderr' key of the result's `metadata` attribute".format(
                         l
                     )
                 )

tests/test_core.py

@@ -1191,7 +1191,7 @@ def test_out_forcing(package):
     )
 
 
-def test_format_config():
+def test_format_config(package):
     inp = {
         "out_temperature": True,
         "out_allowdynamicvars": False,
@@ -1204,50 +1204,54 @@ def test_format_config():
         "out_keydata1_vars": ["DAT_SURF_TEMP"],
         "out_dynamic_vars": ["DAT_SURF_TEMP"],
     }
-    m = MAGICC7()
 
-    res = m._convert_out_config_flags_to_integers(inp)
+    res = package._convert_out_config_flags_to_integers(inp)
     assert exp == res
 
 
 @pytest.mark.slow
-def test_limit_output():
-    # Check that we can run the model an only output a single variable
-    try:
-        with MAGICC7() as m:
-            m.set_output_variables(write_binary=True, write_ascii=False)
-            res = m.run(out_dynamic_vars=["DAT_SURFACE_TEMP"])
+def test_limit_output(package):
+    # Check that we can run the model and only output a single variable
+    package.set_output_variables(write_binary=True, write_ascii=False)
+    if package.version == 6:
+        # MAGICC6 doesn't have this capability
+        with pytest.raises(CalledProcessError):
+            package.run(out_dynamic_vars=["DAT_SURFACE_TEMP"])
+        return
 
-            assert listdir(m.out_dir) == ["DAT_SURFACE_TEMP.BINOUT"]
-            assert res["variable"].unique() == ["Surface Temperature"]
-    except FileNotFoundError:
-        pytest.skip("MAGICC7 not installed")
+    res = package.run(out_dynamic_vars=["DAT_SURFACE_TEMP"])
+    assert listdir(package.out_dir) == ["DAT_SURFACE_TEMP.BINOUT"]
+    assert res["variable"].unique() == ["Surface Temperature"]
 
 
 @pytest.mark.slow
 def test_stderr_debug(package):
+    if package.version == 6:
+        # MAGICC6 doesn't have this capability
+        with pytest.raises(ValueError):
+            res = package.run(debug=True, only=["Surface Temperature"])
+        return
+
     res = package.run(debug=True, only=["Surface Temperature"])
 
     assert "stderr" in res.metadata
-
-    if package.version >= 7:
-        assert "<DEBUG>" in res.metadata["stderr"]
-
-    # Also check that the debug flag takes preference
-    res = package.run(debug=True, verbose=True, only=["Surface Temperature"])
-    if package.version >= 7:
-        assert "<DEBUG>" in res.metadata["stderr"]
+    assert "<DEBUG>" in res.metadata["stderr"]
 
 
 @pytest.mark.slow
 def test_stderr_verbose(package):
-    res = package.run(verbose=True, only=["Surface Temperature"])
+    if package.version == 6:
+        # MAGICC6 doesn't have this capability
+        with pytest.raises(ValueError):
+            res = package.run(debug=True, only=["Surface Temperature"])
+        return
+
+    res = package.run(debug="verbose", only=["Surface Temperature"])
 
     assert "stderr" in res.metadata
 
-    if package.version >= 7:
-        assert "<DEBUG>" not in res.metadata["stderr"]
-        assert "<INFO>" in res.metadata["stderr"]
+    assert "<DEBUG>" not in res.metadata["stderr"]
+    assert "<INFO>" in res.metadata["stderr"]
 
 
 @pytest.mark.slow
@@ -1285,7 +1289,7 @@ def run(*args, **kwargs):
         with MAGICC7() as m:
             if raises:
                 with pytest.warns(
-                    UserWarning, match=r"magicc logged an {} message*".format(level)
+                    UserWarning, match=r"magicc logged a {} message*".format(level)
                 ):
                     m.run(out_dynamic_vars=["DAT_SURFACE_TEMP"])
             else: