refact(op): simplify hierarchy, Op fully abstract

Author: ankostis

graphtik/netop.py

@@ -64,9 +64,11 @@ def __init__(
         :raises ValueError:
             see :meth:`narrow()`
         """
+        self.name = name
+        self.inputs = inputs
+        self.provides = outputs
         self.net = net.pruned(inputs, outputs)
         ## Set data asap, for debugging, although `prune()` will reset them.
-        super().__init__(name, inputs, outputs)
         self.set_execution_method(method)
         self.set_overwrites_collector(overwrites_collector)
 

graphtik/op.py

@@ -5,7 +5,7 @@
 import abc
 import logging
 from collections import abc as cabc
-from typing import Callable
+from typing import Callable, Collection, Tuple, Union
 
 from boltons.setutils import IndexedSet as iset
 
@@ -43,43 +43,6 @@ def reparse_operation_data(name, needs, provides):
 class Operation(abc.ABC):
     """An abstract class representing a data transformation by :meth:`.compute()`."""
 
-    def __init__(self, name, needs=None, provides=None):
-        """
-        Create a new layer instance.
-        Names may be given to this layer and its inputs and outputs. This is
-        important when connecting layers and data in a Network object, as the
-        names are used to construct the graph.
-
-        :param str name:
-            The name the operation (e.g. conv1, conv2, etc..)
-
-        :param list needs:
-            Names of input data objects this layer requires.
-
-        :param list provides:
-            Names of output data objects this provides.
-
-        """
-
-        # (Optional) names for this layer, and the data it needs and provides
-        self.name = name
-        self.needs = needs
-        self.provides = provides
-
-    def __eq__(self, other):
-        """
-        Operation equality is based on name of layer.
-        (__eq__ and __hash__ must be overridden together)
-        """
-        return bool(self.name is not None and self.name == getattr(other, "name", None))
-
-    def __hash__(self):
-        """
-        Operation equality is based on name of layer.
-        (__eq__ and __hash__ must be overridden together)
-        """
-        return hash(self.name)
-
     @abc.abstractmethod
     def compute(self, named_inputs, outputs=None):
         """
@@ -89,23 +52,12 @@ def compute(self, named_inputs, outputs=None):
         End-users should simply call the operation with `named_inputs` as kwargs.
 
         :param list named_inputs:
-            A list of :class:`Data` objects on which to run the layer's
-            feed-forward computation.
+            the input values with which to feed the computation.
         :returns list:
             Should return a list values representing
             the results of running the feed-forward computation on
             ``inputs``.
         """
-        pass
-
-    def __repr__(self):
-        """
-        Display more informative names for the Operation class
-        """
-        clsname = type(self).__name__
-        needs = aslist(self.needs, "needs")
-        provides = aslist(self.provides, "provides")
-        return f"{clsname}({self.name!r}, needs={needs!r}, provides={provides!r})"
 
 
 class FunctionalOperation(Operation):
@@ -119,19 +71,38 @@ def __init__(
         self,
         fn: Callable,
         name,
-        needs=None,
-        provides=None,
+        needs: Union[Collection, str] = None,
+        provides: Union[Collection, str] = None,
         *,
-        parents=None,
+        parents: Tuple = None,
         returns_dict=None,
     ):
+        """
+        Build a new operation out of some function and its requirements.
+
+        :param name:
+            a name for the operation (e.g. `'conv1'`, `'sum'`, etc..);
+            it will be prefixed by `parents`.
+        :param needs:
+            Names of input data objects this operation requires.
+        :param provides:
+            Names of output data objects this provides.
+        :param parent:
+            a tuple wth the names of the parents, prefixing `name`,
+            but also kept for equality/hash check.
+
+        """
+        ## Set op-data early, for repr() to work on errors.
         self.fn = fn
+        self.name = name
+        self.needs = needs
+        self.provides = provides
         self.parents = parents
         self.returns_dict = returns_dict
-        ## Set op-data early, for repr() to work on errors.
-        super().__init__(name, needs, provides)
         if not fn or not callable(fn):
-            raise ValueError(f"Operation was not provided with a callable: {self.fn}")
+            raise ValueError(
+                f"Operation was not provided with a callable: {fn}\n  {self}"
+            )
         if parents and not isinstance(parents, tuple):
             raise ValueError(
                 f"Operation `parents` must be tuple, was {parents}\n {self}"
@@ -143,21 +114,15 @@ def __init__(
         )
 
     def __eq__(self, other):
-        """
-        Operation equality is based on name of layer.
-        (__eq__ and __hash__ must be overridden together)
-        """
+        """Operation identity is based on `name` and `parents`."""
         return bool(
             self.name is not None
             and self.name == getattr(other, "name", None)
             and self.parents == getattr(other, "parents", None)
         )
 
     def __hash__(self):
-        """
-        Operation equality is based on name of layer.
-        (__eq__ and __hash__ must be overridden together)
-        """
+        """Operation identity is based on `name` and `parents`."""
         return hash(self.name) ^ hash(self.parents)
 
     def __repr__(self):

test/test_base.py

@@ -163,7 +163,9 @@ def test_jetsam_sites_screaming_func(acallable, expected_jetsam):
 
 class DummyOperation(op.Operation):
     def __init__(self):
-        super().__init__("", (), ("a"))
+        self.name = ("",)
+        self.needs = ()
+        self.provides = ("a",)
 
     def compute(self, named_inputs, outputs=None):
         pass

test/test_op.py

@@ -22,11 +22,6 @@ def opprovides(request):
     return request.param
 
 
-class MyOp(Operation):
-    def compute(self):
-        pass
-
-
 def test_repr_smoke(opname, opneeds, opprovides):
     # Simply check __repr__() does not crash on partial attributes.
     kw = locals().copy()
@@ -35,9 +30,6 @@ def test_repr_smoke(opname, opneeds, opprovides):
     op = operation(**kw)
     str(op)
 
-    op = MyOp(**kw)
-    str(op)
-
 
 def test_repr_returns_dict():
     assert (