From 7f707ecb6104d1a7ec5bea9918d3eafb728d798d Mon Sep 17 00:00:00 2001
From: Benoit Bovy <benbovy@gmail.com>
Date: Mon, 30 Sep 2019 16:49:10 +0200
Subject: [PATCH] Decouple process class from Model (#63)

* embed modified "dataclass" in __xsimlab_cls__ attribute

* fix all existing tests
---
 doc/index.rst                    |   2 +
 doc/testing.rst                  |  36 +++++++
 doc/whats_new.rst                |   3 +
 xsimlab/model.py                 |  15 +--
 xsimlab/process.py               | 168 +++++++++++++++++--------------
 xsimlab/tests/fixture_process.py |   5 +-
 xsimlab/tests/test_formatting.py |   3 +-
 xsimlab/tests/test_model.py      |   7 +-
 xsimlab/tests/test_process.py    |  80 +++++++++++----
 xsimlab/tests/test_utils.py      |   7 +-
 xsimlab/variable.py              |  22 +++-
 11 files changed, 228 insertions(+), 120 deletions(-)
 create mode 100644 doc/testing.rst

diff --git a/doc/index.rst b/doc/index.rst
index 4bf27678..2374d22d 100644
--- a/doc/index.rst
+++ b/doc/index.rst
@@ -35,6 +35,7 @@ Documentation index
 * :doc:`create_model`
 * :doc:`inspect_model`
 * :doc:`run_model`
+* :doc:`testing`
 
 .. toctree::
    :maxdepth: 1
@@ -45,6 +46,7 @@ Documentation index
    create_model
    inspect_model
    run_model
+   testing
 
 **Help & Reference**
 
diff --git a/doc/testing.rst b/doc/testing.rst
new file mode 100644
index 00000000..d059ad1a
--- /dev/null
+++ b/doc/testing.rst
@@ -0,0 +1,36 @@
+.. _testing:
+
+Testing
+=======
+
+Testing and/or debugging the logic implemented in process classes can
+be achieved easily just by instantiating them. The xarray-simlab
+framework is not invasive and process classes can be used like other,
+regular Python classes.
+
+.. ipython:: python
+   :suppress:
+
+    import sys
+    sys.path.append('scripts')
+    from advection_model import InitUGauss
+
+Here is an example with one of the process classes created in section
+:doc:`create_model`:
+
+.. ipython:: python
+
+    import numpy as np
+    import matplotlib.pyplot as plt
+    gauss = InitUGauss(loc=0.3, scale=0.1, x=np.arange(0, 1.5, 0.01))
+    gauss.initialize()
+    @savefig gauss.png width=50%
+    plt.plot(gauss.x, gauss.u);
+
+Like for any other process class, the parameters of
+``InitUGauss.__init__`` correspond to each of the variables declared
+in that class with either ``intent='in'`` or ``intent='inout'``. Those
+parameters are "keyword only" (see `PEP 3102`_), i.e., it is not
+possible to set these as positional arguments.
+
+.. _`PEP 3102`: https://www.python.org/dev/peps/pep-3102/
diff --git a/doc/whats_new.rst b/doc/whats_new.rst
index f8cc5769..ec59bea0 100644
--- a/doc/whats_new.rst
+++ b/doc/whats_new.rst
@@ -37,6 +37,9 @@ Enhancements
   to the (runtime) methods defined in process classes (:issue:`59`).
 - Better documentation with a minimal, yet illustrative example based
   on Game of Life (:issue:`61`).
+- A class decorated with ``process`` can now be instantiated
+  independently of any Model object. This is very useful for testing
+  and debugging (:issue:`63`).
 
 Bug fixes
 ~~~~~~~~~
diff --git a/xsimlab/model.py b/xsimlab/model.py
index 8197c6e1..4896f17d 100644
--- a/xsimlab/model.py
+++ b/xsimlab/model.py
@@ -2,7 +2,7 @@
 from inspect import isclass
 
 from .variable import VarIntent, VarType
-from .process import (ensure_process_decorated, filter_variables,
+from .process import (filter_variables, get_process_cls,
                       get_target_variable, SimulationStage)
 from .utils import AttrMapping, ContextMixin, has_method, variables_dict
 from .formatting import repr_model
@@ -43,7 +43,7 @@ def __init__(self, processes_cls):
         self._processes_cls = processes_cls
         self._processes_obj = {k: cls() for k, cls in processes_cls.items()}
 
-        self._reverse_lookup = self._get_reverse_lookup(processes_cls)
+        self._reverse_lookup = self._get_reverse_lookup(self._processes_cls)
 
         self._input_vars = None
 
@@ -391,20 +391,13 @@ def __init__(self, processes):
 
         Raises
         ------
-        :exc:`TypeError`
-            If values in ``processes`` are not classes.
         :exc:`NoteAProcessClassError`
             If values in ``processes`` are not classes decorated with
             :func:`process`.
 
         """
-        for cls in processes.values():
-            if not isclass(cls):
-                raise TypeError("Dictionary values must be classes, "
-                                "found {}".format(cls))
-            ensure_process_decorated(cls)
-
-        builder = _ModelBuilder(processes)
+        builder = _ModelBuilder({k: get_process_cls(v)
+                                 for k, v in processes.items()})
 
         builder.bind_processes(self)
         builder.set_process_keys()
diff --git a/xsimlab/process.py b/xsimlab/process.py
index 019691ca..954e1bdc 100644
--- a/xsimlab/process.py
+++ b/xsimlab/process.py
@@ -20,10 +20,17 @@ class NotAProcessClassError(ValueError):
     pass
 
 
-def ensure_process_decorated(cls):
-    if not getattr(cls, "__xsimlab_process__", False):
-        raise NotAProcessClassError("{cls!r} is not a "
-                                    "process-decorated class.".format(cls=cls))
+def _get_embedded_process_cls(cls):
+    if getattr(cls, "__xsimlab_process__", False):
+        return cls
+
+    else:
+        try:
+            return cls.__xsimlab_cls__
+        except AttributeError:
+            raise NotAProcessClassError("{cls!r} is not a "
+                                        "process-decorated class."
+                                        .format(cls=cls))
 
 
 def get_process_cls(obj_or_cls):
@@ -32,22 +39,16 @@ def get_process_cls(obj_or_cls):
     else:
         cls = obj_or_cls
 
-    ensure_process_decorated(cls)
-
-    return cls
+    return _get_embedded_process_cls(cls)
 
 
 def get_process_obj(obj_or_cls):
     if inspect.isclass(obj_or_cls):
         cls = obj_or_cls
-        obj = cls()
     else:
         cls = type(obj_or_cls)
-        obj = obj_or_cls
 
-    ensure_process_decorated(cls)
-
-    return obj
+    return _get_embedded_process_cls(cls)()
 
 
 def filter_variables(process, var_type=None, intent=None, group=None,
@@ -137,46 +138,6 @@ def get_target_variable(var):
     return target_process_cls, target_var
 
 
-def _attrify_class(cls):
-    """Return a `cls` after having passed through :func:`attr.attrs`.
-
-    This pulls out and converts `attr.ib` declared as class attributes
-    into :class:`attr.Attribute` objects and it also adds
-    dunder-methods such as `__init__`.
-
-    The following instance attributes are also defined with None or
-    empty values (proper values will be set later at model creation):
-
-    __xsimlab_model__ : obj
-        :class:`Model` instance to which the process instance is attached.
-    __xsimlab_name__ : str
-        Name given for this process in the model.
-    __xsimlab_store__ : dict or object
-        Simulation data store.
-    __xsimlab_store_keys__ : dict
-        Dictionary that maps variable names to their corresponding key
-        (or list of keys for group variables) in the store.
-        Such keys consist of pairs like `('foo', 'bar')` where
-        'foo' is the name of any process in the same model and 'bar' is
-        the name of a variable declared in that process.
-    __xsimlab_od_keys__ : dict
-        Dictionary that maps variable names to the location of their target
-        on-demand variable (or a list of locations for group variables).
-        Locations are tuples like store keys.
-
-    """
-    def init_process(self):
-        self.__xsimlab_model__ = None
-        self.__xsimlab_name__ = None
-        self.__xsimlab_store__ = None
-        self.__xsimlab_store_keys__ = {}
-        self.__xsimlab_od_keys__ = {}
-
-    setattr(cls, '__attrs_post_init__', init_process)
-
-    return attr.attrs(cls)
-
-
 def _make_property_variable(var):
     """Create a property for a variable or a foreign variable (after
     some sanity checks).
@@ -400,11 +361,42 @@ def execute(self, obj, stage, runtime_context):
             return executor.execute(obj, runtime_context)
 
 
+def _process_cls_init(obj):
+    """Set the following instance attributes with None or empty values
+    (proper values will be set later at model creation):
+
+    __xsimlab_model__ : obj
+        :class:`Model` instance to which the process instance is attached.
+    __xsimlab_name__ : str
+        Name given for this process in the model.
+    __xsimlab_store__ : dict or object
+        Simulation data store.
+    __xsimlab_store_keys__ : dict
+        Dictionary that maps variable names to their corresponding key
+        (or list of keys for group variables) in the store.
+        Such keys consist of pairs like `('foo', 'bar')` where
+        'foo' is the name of any process in the same model and 'bar' is
+        the name of a variable declared in that process.
+    __xsimlab_od_keys__ : dict
+        Dictionary that maps variable names to the location of their target
+        on-demand variable (or a list of locations for group variables).
+        Locations are tuples like store keys.
+
+    """
+    obj.__xsimlab_model__ = None
+    obj.__xsimlab_name__ = None
+    obj.__xsimlab_store__ = None
+    obj.__xsimlab_store_keys__ = {}
+    obj.__xsimlab_od_keys__ = {}
+
+
 class _ProcessBuilder:
-    """Used to iteratively create a new process class.
+    """Used to iteratively create a new process class from an existing
+    "dataclass", i.e., a class decorated with ``attr.attrs``.
 
-    The original class must be already "attr-yfied", i.e., it must
-    correspond to a class returned by `attr.attrs`.
+    The process class is a direct child of the given dataclass, with
+    attributes (fields) redefined and properties created so that it
+    can be used within a model.
 
     """
     _make_prop_funcs = {
@@ -415,32 +407,59 @@ class _ProcessBuilder:
     }
 
     def __init__(self, attr_cls):
-        self._cls = attr_cls
-        self._cls.__xsimlab_process__ = True
-        self._cls.__xsimlab_executor__ = _ProcessExecutor(self._cls)
-        self._cls_dict = {}
+        self._base_cls = attr_cls
+        self._p_cls_dict = {}
 
-    def add_properties(self, var_type):
-        make_prop_func = self._make_prop_funcs[var_type]
+    def _reset_attributes(self):
+        new_attributes = OrderedDict()
 
-        for var_name, var in filter_variables(self._cls, var_type).items():
-            self._cls_dict[var_name] = make_prop_func(var)
+        for k, attrib in attr.fields_dict(self._base_cls).items():
+            new_attributes[k] = attr.attrib(
+                metadata=attrib.metadata,
+                validator=attrib.validator,
+                default=attr.NOTHING,
+                init=False,
+                cmp=False,
+                repr=False
+            )
 
-    def add_repr(self):
-        self._cls_dict['__repr__'] = repr_process
+        return new_attributes
+
+    def _make_process_subclass(self):
+        p_cls = attr.make_class(self._base_cls.__name__,
+                                self._reset_attributes(),
+                                bases=(self._base_cls,),
+                                init=False,
+                                repr=False)
+
+        setattr(p_cls, '__init__', _process_cls_init)
+        setattr(p_cls, '__repr__', repr_process)
+        setattr(p_cls, '__xsimlab_process__', True)
+        setattr(p_cls, '__xsimlab_executor__', _ProcessExecutor(p_cls))
+
+        return p_cls
+
+    def add_properties(self):
+        for var_name, var in attr.fields_dict(self._base_cls).items():
+            var_type = var.metadata.get('var_type')
+
+            if var_type is not None:
+                make_prop_func = self._make_prop_funcs[var_type]
+
+                self._p_cls_dict[var_name] = make_prop_func(var)
 
     def render_docstrings(self):
-        # self._cls_dict['__doc__'] = "Process-ified class."
+        # self._p_cls_dict['__doc__'] = "Process-ified class."
         raise NotImplementedError("autodoc is not yet implemented.")
 
     def build_class(self):
-        cls = self._cls
+        p_cls = self._make_process_subclass()
 
         # Attach properties (and docstrings)
-        for name, value in self._cls_dict.items():
-            setattr(cls, name, value)
+        for name, value in self._p_cls_dict.items():
+            setattr(p_cls, name, value)
 
-        return cls
+        return p_cls
 
 
 def process(maybe_cls=None, autodoc=False):
@@ -475,19 +494,18 @@ def process(maybe_cls=None, autodoc=False):
 
     """
     def wrap(cls):
-        attr_cls = _attrify_class(cls)
+        attr_cls = attr.attrs(cls)
 
         builder = _ProcessBuilder(attr_cls)
 
-        for var_type in VarType:
-            builder.add_properties(var_type)
+        builder.add_properties()
 
         if autodoc:
             builder.render_docstrings()
 
-        builder.add_repr()
+        setattr(attr_cls, '__xsimlab_cls__', builder.build_class())
 
-        return builder.build_class()
+        return attr_cls
 
     if maybe_cls is None:
         return wrap
diff --git a/xsimlab/tests/fixture_process.py b/xsimlab/tests/fixture_process.py
index 1271ab02..579cda9b 100644
--- a/xsimlab/tests/fixture_process.py
+++ b/xsimlab/tests/fixture_process.py
@@ -4,6 +4,7 @@
 import pytest
 
 import xsimlab as xs
+from xsimlab.process import get_process_obj
 
 
 @xs.process
@@ -49,7 +50,7 @@ def compute_od_var(self):
 
 @pytest.fixture
 def example_process_obj():
-    return ExampleProcess()
+    return get_process_obj(ExampleProcess)
 
 
 @pytest.fixture(scope='session')
@@ -85,7 +86,7 @@ def in_var_details():
 
 
 def _init_process(p_cls, p_name, model, store, store_keys=None, od_keys=None):
-    p_obj = p_cls()
+    p_obj = get_process_obj(p_cls)
     p_obj.__xsimlab_name__ = p_name
     p_obj.__xsimlab_model__ = model
     p_obj.__xsimlab_store__ = store
diff --git a/xsimlab/tests/test_formatting.py b/xsimlab/tests/test_formatting.py
index 065e474f..9585394d 100644
--- a/xsimlab/tests/test_formatting.py
+++ b/xsimlab/tests/test_formatting.py
@@ -4,6 +4,7 @@
 from xsimlab.formatting import (maybe_truncate, pretty_print,
                                 repr_process, repr_model,
                                 var_details, wrap_indent)
+from xsimlab.process import get_process_obj
 
 
 def test_maybe_truncate():
@@ -60,7 +61,7 @@ def run_step(self):
         run_step
     """)
 
-    assert repr_process(Dummy()) == expected
+    assert repr_process(get_process_obj(Dummy)) == expected
 
 
 def test_model_repr(simple_model, simple_model_repr):
diff --git a/xsimlab/tests/test_model.py b/xsimlab/tests/test_model.py
index fa880fb5..15b9bd05 100644
--- a/xsimlab/tests/test_model.py
+++ b/xsimlab/tests/test_model.py
@@ -1,6 +1,7 @@
 import pytest
 
 import xsimlab as xs
+from xsimlab.process import get_process_cls
 from xsimlab.tests.fixture_model import AddOnDemand, InitProfile, Profile
 
 
@@ -113,7 +114,7 @@ class InheritedProfile(Profile):
         new_model = model.update_processes(
             {'profile': InheritedProfile})
 
-        assert type(new_model['profile']) is InheritedProfile
+        assert type(new_model['profile']) is get_process_cls(InheritedProfile)
         assert isinstance(new_model['profile'], Profile)
 
         with pytest.raises(ValueError) as excinfo:
@@ -125,10 +126,6 @@ class InheritedProfile(Profile):
 class TestModel:
 
     def test_constructor(self):
-        with pytest.raises(TypeError) as excinfo:
-            xs.Model({'init_profile': InitProfile()})
-        assert "values must be classes" in str(excinfo.value)
-
         with pytest.raises(KeyError) as excinfo:
             xs.Model({'init_profile': InitProfile})
         assert "Process class 'Profile' missing" in str(excinfo.value)
diff --git a/xsimlab/tests/test_process.py b/xsimlab/tests/test_process.py
index 2018007b..455f9ec5 100644
--- a/xsimlab/tests/test_process.py
+++ b/xsimlab/tests/test_process.py
@@ -1,10 +1,11 @@
 from io import StringIO
+import inspect
 
 import pytest
 
 import xsimlab as xs
 from xsimlab.variable import VarIntent, VarType
-from xsimlab.process import (ensure_process_decorated, filter_variables,
+from xsimlab.process import (filter_variables,
                              get_process_cls, get_process_obj,
                              get_target_variable, NotAProcessClassError,
                              process_info, variable_info)
@@ -12,23 +13,30 @@
 from xsimlab.tests.fixture_process import ExampleProcess, SomeProcess
 
 
-def test_ensure_process_decorated():
+def test_get_process_cls(example_process_obj):
+    p_cls = get_process_cls(ExampleProcess)
+    assert get_process_cls(example_process_obj) is p_cls
+
+
+def test_get_process_obj(example_process_obj):
+    p_cls = get_process_cls(ExampleProcess)
+    assert type(get_process_obj(ExampleProcess)) is p_cls
+
+    # get_process_obj returns a new instance
+    assert get_process_obj(example_process_obj) is not example_process_obj
+
+
+def test_get_process_raise():
     class NotAProcess:
         pass
 
     with pytest.raises(NotAProcessClassError) as excinfo:
-        ensure_process_decorated(NotAProcess)
+        get_process_cls(NotAProcess)
     assert "is not a process-decorated class" in str(excinfo.value)
 
-
-def test_get_process_cls(example_process_obj):
-    assert get_process_cls(ExampleProcess) is ExampleProcess
-    assert get_process_cls(example_process_obj) is ExampleProcess
-
-
-def test_get_process_obj(example_process_obj):
-    assert get_process_obj(example_process_obj) is example_process_obj
-    assert type(get_process_obj(ExampleProcess)) is ExampleProcess
+    with pytest.raises(NotAProcessClassError) as excinfo:
+        get_process_obj(NotAProcess)
+    assert "is not a process-decorated class" in str(excinfo.value)
 
 
 @pytest.mark.parametrize('kwargs,expected', [
@@ -50,26 +58,30 @@ def test_filter_variables(kwargs, expected):
     assert set(filter_variables(ExampleProcess, **kwargs)) == expected
 
 
-@pytest.mark.parametrize('var_name,expected_p_cls,expected_var_name', [
+@pytest.mark.parametrize('var_name,expected_cls,expected_var_name', [
     ('in_var', ExampleProcess, 'in_var'),
     ('in_foreign_var', SomeProcess, 'some_var'),
     ('in_foreign_var2', SomeProcess, 'some_var')  # test foreign of foreign
 ])
-def test_get_target_variable(var_name, expected_p_cls, expected_var_name):
-    var = variables_dict(ExampleProcess)[var_name]
+def test_get_target_variable(var_name, expected_cls, expected_var_name):
+    _ExampleProcess = get_process_cls(ExampleProcess)
+    expected_p_cls = get_process_cls(expected_cls)
+
+    var = variables_dict(_ExampleProcess)[var_name]
     expected_var = variables_dict(expected_p_cls)[expected_var_name]
 
-    actual_p_cls, actual_var = get_target_variable(var)
+    actual_cls, actual_var = get_target_variable(var)
 
-    if expected_p_cls is ExampleProcess:
-        assert actual_p_cls is None
+    if expected_p_cls is _ExampleProcess:
+        assert actual_cls is None
     else:
+        actual_p_cls = get_process_cls(actual_cls)
         assert actual_p_cls is expected_p_cls
 
     assert actual_var is expected_var
 
 
-@pytest.mark.parametrize('p_cls,var_name,prop_is_read_only', [
+@pytest.mark.parametrize('cls,var_name,prop_is_read_only', [
     (ExampleProcess, 'in_var', True),
     (ExampleProcess, 'in_foreign_var', True),
     (ExampleProcess, 'group_var', True),
@@ -78,7 +90,9 @@ def test_get_target_variable(var_name, expected_p_cls, expected_var_name):
     (ExampleProcess, 'out_var', False),
     (ExampleProcess, 'out_foreign_var', False)
 ])
-def test_process_properties_readonly(p_cls, var_name, prop_is_read_only):
+def test_process_properties_readonly(cls, var_name, prop_is_read_only):
+    p_cls = get_process_cls(cls)
+
     if prop_is_read_only:
         assert getattr(p_cls, var_name).fset is None
     else:
@@ -112,7 +126,9 @@ def test_process_properties_docstrings(in_var_details):
     # order of lines in string is not ensured (printed from a dictionary)
     to_lines = lambda details_str: sorted(details_str.split('\n'))
 
-    assert to_lines(ExampleProcess.in_var.__doc__) == to_lines(in_var_details)
+    _ExampleProcess = get_process_cls(ExampleProcess)
+
+    assert to_lines(_ExampleProcess.in_var.__doc__) == to_lines(in_var_details)
 
 
 def test_process_properties_values(processes_with_store):
@@ -196,6 +212,28 @@ class Dummy:
             pass
 
 
+def test_process_no_model():
+    params = inspect.signature(ExampleProcess.__init__).parameters
+
+    expected_params = ['self', 'in_var', 'inout_var', 'in_foreign_var',
+                       'in_foreign_var2', 'in_foreign_od_var', 'group_var']
+
+    assert list(params.keys()) == expected_params
+
+    @xs.process
+    class P:
+        invar = xs.variable()
+        outvar = xs.variable(intent='out')
+
+        def initialize(self):
+            self.outvar = self.invar + 2
+
+    p = P(invar=1)
+    p.initialize()
+
+    assert p.outvar == 3
+
+
 def test_process_info(example_process_obj, example_process_repr):
     buf = StringIO()
     process_info(example_process_obj, buf=buf)
diff --git a/xsimlab/tests/test_utils.py b/xsimlab/tests/test_utils.py
index 5dab4d01..43a42f74 100644
--- a/xsimlab/tests/test_utils.py
+++ b/xsimlab/tests/test_utils.py
@@ -2,6 +2,7 @@
 import pytest
 
 from xsimlab import utils
+from xsimlab.process import get_process_cls
 from xsimlab.tests.fixture_process import ExampleProcess
 
 
@@ -13,8 +14,10 @@ def test_variables_dict():
 
 
 def test_has_method():
-    assert utils.has_method(ExampleProcess(), 'compute_od_var')
-    assert not utils.has_method(ExampleProcess(), 'invalid_meth')
+    _ExampleProcess = get_process_cls(ExampleProcess)
+
+    assert utils.has_method(_ExampleProcess(), 'compute_od_var')
+    assert not utils.has_method(_ExampleProcess(), 'invalid_meth')
 
 
 def test_maybe_to_list():
diff --git a/xsimlab/variable.py b/xsimlab/variable.py
index 779666c3..af29620e 100644
--- a/xsimlab/variable.py
+++ b/xsimlab/variable.py
@@ -132,8 +132,15 @@ def variable(dims=(), intent='in', group=None, default=attr.NOTHING,
                 'attrs': attrs or {},
                 'description': description}
 
+    if VarIntent(intent) == VarIntent.OUT:
+        _init = False
+        _repr = False
+    else:
+        _init = True
+        _repr = True
+
     return attr.attrib(metadata=metadata, default=default, validator=validator,
-                       init=False, cmp=False, repr=False)
+                       init=_init, cmp=False, repr=_repr, kw_only=True)
 
 
 def on_demand(dims=(), group=None, description='', attrs=None):
@@ -229,7 +236,15 @@ def foreign(other_process_cls, var_name, intent='in'):
                 'intent': VarIntent(intent),
                 'description': description}
 
-    return attr.attrib(metadata=metadata, init=False, cmp=False, repr=False)
+    if VarIntent(intent) == VarIntent.OUT:
+        _init = False
+        _repr = False
+    else:
+        _init = True
+        _repr = True
+
+    return attr.attrib(metadata=metadata, init=_init, cmp=False, repr=_repr,
+                       kw_only=True)
 
 
 def group(name):
@@ -260,4 +275,5 @@ def group(name):
                 'intent': VarIntent.IN,
                 'description': description}
 
-    return attr.attrib(metadata=metadata, init=False, cmp=False, repr=False)
+    return attr.attrib(metadata=metadata, init=True, cmp=False, repr=True,
+                       default=tuple(), kw_only=True)