Merge pull request #451 from iiasa/enh/2022-W30

Miscellaneous improvements for 2022-W30

RELEASE_NOTES.rst

@@ -4,6 +4,10 @@ Next release
 All changes
 -----------
 
+- Optionally tolerate failures to add individual items in :func:`.store_ts` reporting computation (:pull:`451`); use ``timeseries_only=True`` in check-out to function with :class:`.Scenario` with solution data stored.
+- Bugfix: :class:`.Config` squashed configuration values read from :file:`config.json`, if the respective keys were registered in downstream packages, e.g. :mod:`message_ix`.
+  Allow the values loaded from file to persist (:pull:`451`).
+- Adjust to genno 1.12 and set this as the minimum required version (:pull:`451`).
 - Add :meth:`.enforce` to the :class:`~.base.Model` API for enforcing structure/data consistency before :meth:`.Model.solve` (:pull:`450`).
 
 .. _v3.5.0:

doc/api-model.rst

@@ -32,16 +32,22 @@ Model API
 .. currentmodule:: ixmp.model.base
 
 .. autoclass:: ixmp.model.base.Model
-   :members: name, __init__, initialize, initialize_items, run
+   :members: name, __init__, run, initialize, initialize_items, enforce
 
    In the following, the words **required**, **optional**, etc. have specific meanings as described in `IETF RFC 2119 <https://tools.ietf.org/html/rfc2119>`_.
 
-   Model is an **abstract** class; this means it MUST be subclassed.
-   It has two REQURIED methods that MUST be overridden by subclasses:
+   Model is an *abstract* class; this means it **must** be subclassed.
+   It has two **required** methods that **must** be overridden by subclasses:
 
    .. autosummary::
-      name
       __init__
+      run
+
+   The following attributes and methods are **optional** in subclasses.
+   The default implementations are either empty or implement reasonable default behaviour.
+
+   .. autosummary::
+      enforce
       initialize
       initialize_items
-      run
+      name

ixmp/_config.py

@@ -82,25 +82,24 @@ def __getitem__(self, name):
     def __setitem__(self, name, value):
         setattr(self, name.replace(" ", "_"), value)
 
-    def add_field(self, name, type_, default=None):
+    def add_field(self, name, type_, default, **kwargs):
         # Check `name`
         name = name.replace(" ", "_")
-        if name in self.__dataclass_fields__:
+        if (
+            name in self.__dataclass_fields__
+            and "auto" not in self.__dataclass_fields__[name].metadata
+        ):
             raise ValueError(f"configuration key {repr(name)} already defined")
 
         # Create a new data class with an additional field
         new_cls = make_dataclass(
-            "Values", [(name, type_, field(default=default))], bases=(self.__class__,)
+            "Values",
+            [(name, type_, field(default=default, **kwargs))],
+            bases=(self.__class__,),
         )
 
         # Re-use current values and any defaults for the new fields
-        data = asdict(self)
-        try:
-            data[name] = getattr(self, name)
-        except AttributeError:
-            pass
-
-        return new_cls, new_cls(**data)
+        return new_cls, new_cls(**asdict(self))
 
     def delete_field(self, name):
         # Check `name`
@@ -249,9 +248,11 @@ def read(self):
         # Parse JSON and set values
         for key, value in data.items():
             try:
-                self.set(key, value, _strict=True)  # Cast type for registered keys
+                self.set(key, value)  # Cast type for registered keys
             except KeyError:
-                self.set(key, value, _strict=False)  # Tolerate unregistered keys
+                # Automatically register new values
+                self.register(key, type(value), default=None, metadata=dict(auto=True))
+                self.set(key, value)
 
     # Public methods
 
@@ -263,7 +264,7 @@ def keys(self) -> Tuple[str, ...]:
         """Return the names of all registered configuration keys."""
         return self.values.keys()
 
-    def register(self, name: str, type_: type, default: Any = None):
+    def register(self, name: str, type_: type, default: Any = None, **kwargs):
         """Register a new configuration key.
 
         Parameters
@@ -281,7 +282,9 @@ def register(self, name: str, type_: type, default: Any = None):
         ValueError
             if the key `name` is already registered.
         """
-        self._ValuesClass, self.values = self.values.add_field(name, type_, default)
+        self._ValuesClass, self.values = self.values.add_field(
+            name, type_, default, **kwargs
+        )
 
     def unregister(self, name: str) -> None:
         """Unregister and clear the configuration key `name`."""

ixmp/backend/jdbc.py

@@ -805,7 +805,16 @@ def set_data(self, ts, region, variable, data, unit, subannual, meta):
         # Integer so JPype does not produce invalid java.lang.Long.
         jdata = java.LinkedHashMap({java.Integer(k): v for k, v in data.items()})
 
-        self.jindex[ts].addTimeseries(region, variable, subannual, jdata, unit, meta)
+        try:
+            self.jindex[ts].addTimeseries(
+                region, variable, subannual, jdata, unit, meta
+            )
+        except java.IxException as e:
+            match = re.search("node '([^']*)' does not exist in the database", str(e))
+            if match:
+                raise ValueError(f"region = {match.group(1)}") from None
+            else:
+                raise
 
     def set_geo(self, ts, region, variable, subannual, year, value, unit, meta):
         self.jindex[ts].addGeoData(

ixmp/model/base.py

@@ -14,12 +14,14 @@ class ModelError(Exception):
 
 class Model(ABC):
     #: Name of the model.
-    name = "base"
+    name: str = "base"
 
     @abstractmethod
     def __init__(self, name, **kwargs):
         """Constructor.
 
+        **Required.**
+
         Parameters
         ----------
         kwargs :
@@ -38,12 +40,13 @@ def clean_path(cls, value: str) -> str:
     def enforce(scenario):
         """Enforce data consistency in `scenario`.
 
-        Optional. Implementations of :meth:`enforce`:
+        **Optional**; the default implementation does nothing. Subclass implementations
+        of :meth:`enforce`:
 
         - **should** modify the contents of sets and parameters so that `scenario`
           contains structure and data that is consistent with the underlying model.
         - **must not** add or remove sets or parameters; for that, use
-          :meth:`initiatize`.
+          :meth:`initialize`.
 
         :meth:`enforce` is always called by :meth:`run` before the model is run or
         solved; it **may** be called manually at other times.
@@ -58,7 +61,8 @@ def enforce(scenario):
     def initialize(cls, scenario):
         """Set up *scenario* with required items.
 
-        Implementations of :meth:`initialize`:
+        **Optional**; the default implementation does nothing. Subclass implementations
+        of :meth:`initialize`:
 
         - **may** add sets, set elements, and/or parameter values.
         - **may** accept any number of keyword arguments to control behaviour.
@@ -180,10 +184,11 @@ def initialize_items(cls, scenario, items):
     def run(self, scenario):
         """Execute the model.
 
-        Implementations of :meth:`run`:
+        **Required.** Implementations of :meth:`run`:
 
         - **must** call :meth:`enforce`.
 
+
         Parameters
         ----------
         scenario : .Scenario

ixmp/reporting/computations.py

@@ -167,7 +167,7 @@ def map_as_qty(set_df: pd.DataFrame, full_set):
     )
 
 
-def store_ts(scenario, *data):
+def store_ts(scenario, *data, strict: bool = False) -> None:
     """Store time series `data` on `scenario`.
 
     The data is stored using :meth:`.add_timeseries`; `scenario` is checked out and then
@@ -180,14 +180,15 @@ def store_ts(scenario, *data):
     data : pandas.DataFrame or pyam.IamDataFrame
         1 or more objects containing data to store. If :class:`pandas.DataFrame`, the
         data are passed through :func:`to_iamc_layout`.
+    strict: bool
+        If :data:`True` (default :data:`False`), raise an exception if any of `data` are
+        not successfully added. Otherwise, log on level :ref:`ERROR <python:levels>` and
+        continue.
     """
-    # TODO tolerate invalid types/errors on elements of `data`, logging exceptions on
-    #      level ERROR, then continue and commit anyway; add an optional parameter like
-    #      continue_on_error=True to control this behaviour
     import pyam
 
     log.info(f"Store time series data on '{scenario.url}'")
-    scenario.check_out()
+    scenario.check_out(timeseries_only=True)
 
     for order, df in enumerate(data):
         df = (
@@ -197,8 +198,14 @@ def store_ts(scenario, *data):
         )
 
         # Add the data
-        log.info(f"  ← {len(df)} rows")
-        scenario.add_timeseries(df)
+        try:
+            scenario.add_timeseries(df)
+        except Exception as e:
+            log.error(f"Failed with {e!r}:\n{df}")
+            if strict:
+                raise
+        else:
+            log.info(f"  ← {len(df)} rows")
 
     scenario.commit(f"Data added using {__name__}")
 
@@ -222,9 +229,12 @@ def update_scenario(scenario, *quantities, params=[]):
         if not isinstance(qty, pd.DataFrame):
             # Convert a Quantity to a DataFrame
             par_name = qty.name
-            new = qty.to_series().reset_index().rename(columns={par_name: "value"})
-            new["unit"] = "{:~}".format(qty.attrs["_unit"])  # type: ignore [str-format]
-            qty = new
+            qty = (
+                qty.to_series()
+                .reset_index()
+                .rename(columns={par_name: "value"})
+                .assign(unit=f"{qty.units:~}")
+            )
 
         # Add the data
         log.info(f"  {repr(par_name)} ← {len(qty)} rows")

ixmp/reporting/reporter.py

@@ -17,14 +17,7 @@ def __init__(self, *args, **kwargs):
 
         # Append ixmp.reporting.computations to the modules in which the Computer will
         # look up computations names.
-        # genno <= 1.11
-        from ixmp.reporting import computations
-
-        if computations not in self.modules:
-            self.modules.append(computations)
-
-        # TODO use this once genno >= 1.12.0 is released
-        # self.require_compat("ixmp.reporting.computations")
+        self.require_compat("ixmp.reporting.computations")
 
     @classmethod
     def from_scenario(cls, scenario: Scenario, **kwargs) -> "Reporter":

ixmp/tests/reporting/test_computations.py

@@ -1,10 +1,11 @@
 import logging
+import re
 from functools import partial
 
 import pandas as pd
 import pyam
 import pytest
-from genno import Computer, Quantity
+from genno import ComputationError, Computer, Quantity
 from genno.testing import assert_qty_equal
 from pandas.testing import assert_frame_equal
 
@@ -135,3 +136,29 @@ def test_store_ts(request, caplog, test_mp):
     # Input is stored exactly
     assert_frame_equal(expected_1, scen.timeseries(variable="Foo"))
     assert_frame_equal(expected_2, scen.timeseries(variable="Bar"))
+
+    # Data with an unregistered region name
+    c.add("input 3", test_data[0].assign(variable="Foo", region="Moon"))
+    c.add("test 2", store_ts, "target", "input 3")
+
+    # Succeeds with default strict=False
+    caplog.clear()
+    c.get("test 2")
+
+    # A message is logged
+    r = caplog.record_tuples[-1]
+    assert (
+        "ixmp.reporting.computations" == r[0]
+        and logging.ERROR == r[1]
+        and r[2].startswith("Failed with ValueError('region = Moon')")
+    ), caplog.record_tuples
+
+    caplog.clear()
+
+    # with strict=True, the computation fails
+    c.add("test 2", partial(store_ts, strict=True), "target", "input 3")
+    with pytest.raises(
+        ComputationError,
+        match=re.compile("computing 'test 2' using:.*region = Moon", flags=re.DOTALL),
+    ):
+        c.get("test 2")

ixmp/tests/reporting/test_reporter.py

@@ -166,7 +166,6 @@ def test_cli(ixmp_cli, test_mp, test_data_path):
     # TODO warning should be logged
 
     # Reporting produces the expected command-line output
-
     assert re.match(
         "i          j       "  # Trailing whitespace
         r"""
@@ -176,8 +175,7 @@ def test_cli(ixmp_cli, test_mp, test_data_path):
 seattle    chicago     1\.7
            new-york    2\.5
            topeka      1\.8
-(Name: value, )?dtype: float64
-""",
+(Name: value, )?dtype: float64(, units: dimensionsless)?""",
         result.output,
     )
 

setup.cfg

@@ -30,7 +30,7 @@ zip_safe = True
 include_package_data = True
 install_requires =
     click
-    genno >= 1.11.0
+    genno >= 1.12.0
     JPype1 >= 1.2.1
     openpyxl
     pandas >= 1.2