ENH(OP): improve Results/Provides validations, ...

+ enh: validations before were missing len-mismatching.
+ test: +TCs for validation error-msgs.
+ enh: +JETSAM var: `results` --> `results_fn` + `results_op`

Author: ankostis

docs/source/plotting.rst

@@ -89,8 +89,9 @@ with the folllowing properties, as a debug aid:
  'outputs': None,
  'plan': ExecutionPlan(inputs=('a',), outputs=(), steps:
   +--FunctionalOperation(name='screamer', needs=['a'], provides=['foo'], fn='scream')),
- 'provides': ['foo'],
- 'results': None,
+ 'provides': None,
+ 'results_fn': None,
+ 'results_op': None,
  'solution': {'a': None}}
 
 
@@ -123,9 +124,11 @@ The following annotated attributes *might* have meaningfull value on an exceptio
     the names eventually the graph needed from the operation;
     a subset of the above, and not always what has been declared in the operation.
 
-``results``
-    the values dict, if any; it maybe a *zip* of the provides
-    with the actual returned values of the function, ot the raw results.
+``fn_results``
+    the raw results of the operation's fuction, if any
+
+``op_results``
+    the results, always a dictionary, as matched with operation's `provides`
 
 ``executed```
     a set with the operation nodes & instructions executed till the error happened.

graphtik/op.py

@@ -3,11 +3,16 @@
 """About operation nodes (but not net-ops to break cycle)."""
 
 import abc
-from collections.abc import Hashable
+import logging
+from collections.abc import Hashable, Iterable, Mapping
+
+from boltons.setutils import IndexedSet as iset
 
 from .base import Plotter, aslist, jetsam
 from .modifiers import optional, sideffect
 
+log = logging.getLogger(__name__)
+
 
 def reparse_operation_data(name, needs, provides):
     """
@@ -128,12 +133,44 @@ def __repr__(self):
             f"provides={provides!r}, fn{returns_dict_marker}={fn_name!r})"
         )
 
-    def _validate_results(self, results: dict, real_provides: list):
-        if set(results) != set(real_provides):
+    def _zip_results_with_provides(self, results, real_provides: iset) -> dict:
+        """Zip results with expected "real" (without sideffects) `provides`."""
+        if not real_provides:  # All outputs were sideffects?
+            if results:
+                ## Do not scream,
+                #  it is common to call a function for its sideffects,
+                # which happens to return an irrelevant value.
+                log.warning(
+                    "Ignoring result(%s) because no `provides` given!\n  %s",
+                    results,
+                    self,
+                )
+            results = {}
+        elif not self.returns_dict:
+            nexpected = len(real_provides)
+
+            if nexpected > 1 and (
+                not isinstance(results, Iterable) or len(results) != nexpected
+            ):
+                raise ValueError(
+                    f"Expected x{nexpected} ITERABLE results, got: {results}"
+                )
+
+            if nexpected == 1:
+                results = [results]
+
+            results = dict(zip(real_provides, results))
+
+        if self.returns_dict:
+            if not isinstance(results, Mapping):
+                raise ValueError(f"Expected dict-results, got: {results}\n  {self}")
+        if set(results) != real_provides:
             raise ValueError(
                 f"Results({results}) mismatched provides({real_provides})!\n  {self}"
             )
 
+        return results
+
     def compute(self, named_inputs, outputs=None) -> dict:
         try:
             args = [
@@ -150,35 +187,27 @@ def compute(self, named_inputs, outputs=None) -> dict:
                 if isinstance(n, optional) and n in named_inputs
             }
 
-            # Don't expect sideffect outputs.
-            provides = [n for n in self.provides if not isinstance(n, sideffect)]
-
-            results = self.fn(*args, **optionals)
-
-            if not provides:
-                # All outputs were sideffects?
-                results = {}
-
-            elif not self.returns_dict:
-                if len(provides) == 1:
-                    results = [results]
-
-                results = dict(zip(provides, results))
+            results_fn = self.fn(*args, **optionals)
 
-            self._validate_results(results, provides)
+            provides = iset(n for n in self.provides if not isinstance(n, sideffect))
+            results_op = self._zip_results_with_provides(results_fn, provides)
 
             if outputs:
                 outputs = set(n for n in outputs if not isinstance(n, sideffect))
-                results = {key: val for key, val in results.items() if key in outputs}
+                # Ignore sideffect outputs.
+                results_op = {
+                    key: val for key, val in results_op.items() if key in outputs
+                }
 
-            return results
+            return results_op
         except Exception as ex:
             jetsam(
                 ex,
                 locals(),
                 "outputs",
                 "provides",
-                "results",
+                "results_fn",
+                "results_op",
                 operation="self",
                 args=lambda locs: {
                     "args": locs.get("args"),

test/test_base.py

@@ -138,7 +138,7 @@ class Op:
                 operation(name="test", needs=["a"], provides=["b"])(_scream).compute,
                 named_inputs={"a": 1},
             ),
-            "outputs provides results operation args".split(),
+            "outputs provides results_fn results_op operation args".split(),
         ),
         (
             fnt.partial(
@@ -170,7 +170,7 @@ def test_jetsam_sites_screaming_func(acallable, expected_jetsam):
                 operation(name="test", needs=["a"], provides=["b"])(_scream).compute,
                 named_inputs=None,
             ),
-            "outputs provides results operation args".split(),
+            "outputs provides results_fn results_op operation args".split(),
         ),
         (
             fnt.partial(

test/test_graphtik.py

@@ -10,8 +10,14 @@
 import pytest
 
 import graphtik.network as network
-from graphtik import (AbortedException, abort_run, compose, operation,
-                      optional, sideffect)
+from graphtik import (
+    AbortedException,
+    abort_run,
+    compose,
+    operation,
+    optional,
+    sideffect,
+)
 from graphtik.op import Operation
 
 

test/test_op.py

@@ -27,7 +27,7 @@ def compute(self):
         pass
 
 
-def test_operation_repr_smoke(opname, opneeds, opprovides):
+def test_repr_smoke(opname, opneeds, opprovides):
     # Simply check __repr__() does not crash on partial attributes.
     kw = locals().copy()
     kw = {name[2:]: arg for name, arg in kw.items()}
@@ -39,7 +39,7 @@ def test_operation_repr_smoke(opname, opneeds, opprovides):
     str(op)
 
 
-def test_operation_repr_returns_dict():
+def test_repr_returns_dict():
     assert (
         str(operation(lambda: None, returns_dict=True)())
         == "FunctionalOperation(name=None, needs=[], provides=[], fn{}='<lambda>')"
@@ -69,19 +69,47 @@ def test_operation_repr_returns_dict():
         (("", "a", [()]), ValueError("All `provides` must be str")),
     ],
 )
-def test_operation_validation(opargs, exp):
+def test_validation(opargs, exp):
     if isinstance(exp, Exception):
         with pytest.raises(type(exp), match=str(exp)):
             reparse_operation_data(*opargs)
     else:
         assert reparse_operation_data(*opargs) == exp
 
 
-def test_operation_returns_dict():
+def test_returns_dict():
     result = {"a": 1}
 
     op = operation(lambda: result, provides="a", returns_dict=True)()
     assert op.compute({}) == result
 
     op = operation(lambda: 1, provides="a", returns_dict=False)()
     assert op.compute({}) == result
+
+
+@pytest.fixture(params=[None, ["a", "b"]])
+def asked_outputs(request):
+    return request.param
+
+
+@pytest.mark.parametrize(
+    "result", [None, 3.14, (), "", "foobar", ["b", "c", "e"], {"f"}]
+)
+def test_results_validation_iterable_BAD(result, asked_outputs):
+    op = operation(lambda: result, provides=["a", "b"], returns_dict=False)()
+    with pytest.raises(ValueError, match="Expected x2 ITERABLE results"):
+        op.compute({}, outputs=asked_outputs)
+
+
+@pytest.mark.parametrize("result", [None, 3.14, [], "foo", ["b", "c", "e"], {"a", "b"}])
+def test_dict_results_validation_BAD(result, asked_outputs):
+    op = operation(lambda: result, provides=["a", "b"], returns_dict=True)()
+    with pytest.raises(ValueError, match="Expected dict-results"):
+        op.compute({}, outputs=asked_outputs)
+
+
+@pytest.mark.parametrize("result", [{"a": 1}, {"a": 1, "b": 2, "c": 3}])
+def test_dict_results_validation_MISMATCH(result, asked_outputs):
+    op = operation(lambda: result, provides=["a", "b"], returns_dict=True)()
+    with pytest.raises(ValueError, match="mismatched provides"):
+        op.compute({}, outputs=asked_outputs)