Repository: pythological/etuples

Issue:
Use trampolining for evaluation and etuplization
`ExpressionTuple.eval_obj` and `etuplize` are recursive and not scalable, so we should use some sort of TCO/trampolining (e.g. https://github.com/pythological/unification/pull/16).

Discussion:

Patch:
diff --git a/etuples/core.py b/etuples/core.py
index e7bf476..e275cf6 100644
--- a/etuples/core.py
+++ b/etuples/core.py
@@ -1,9 +1,8 @@
 import inspect
 import reprlib
 
-import toolz
-
-from collections.abc import Sequence
+from collections import deque
+from collections.abc import Sequence, Generator
 
 
 etuple_repr = reprlib.Repr()
@@ -11,6 +10,41 @@ etuple_repr.maxstring = 100
 etuple_repr.maxother = 100
 
 
+def trampoline_eval(z, res_filter=None):
+    """Evaluate a stream of generators.
+
+    This implementation consists of a deque that simulates an evaluation stack
+    of generator-produced operations.  We're able to overcome `RecursionError`s
+    this way.
+    """
+
+    if not isinstance(z, Generator):  # pragma: no cover
+        return z
+
+    stack = deque()
+    z_args, z_out = None, None
+    stack.append(z)
+
+    while stack:
+        z = stack[-1]
+        try:
+            z_out = z.send(z_args)
+
+            if res_filter:  # pragma: no cover
+                _ = res_filter(z, z_out)
+
+            if isinstance(z_out, Generator):
+                stack.append(z_out)
+                z_args = None
+            else:
+                z_args = z_out
+
+        except StopIteration:
+            _ = stack.pop()
+
+    return z_out
+
+
 class InvalidExpression(Exception):
     """An exception indicating that an `ExpressionTuple` is not a valid [S-]expression.
 
@@ -35,9 +69,13 @@ class KwdPair(object):
         self.arg = arg
         self.value = value
 
-    @property
-    def eval_obj(self):
-        return KwdPair(self.arg, getattr(self.value, "eval_obj", self.value))
+    def _eval_step(self):
+        if isinstance(self.value, (ExpressionTuple, KwdPair)):
+            value = yield self.value._eval_step()
+        else:
+            value = self.value
+
+        yield KwdPair(self.arg, value)
 
     def __repr__(self):
         return f"{self.__class__.__name__}({repr(self.arg)}, {repr(self.value)})"
@@ -111,28 +149,36 @@ class ExpressionTuple(Sequence):
 
     @property
     def eval_obj(self):
-        """Return the evaluation of this expression tuple.
+        """Return the evaluation of this expression tuple."""
+        return trampoline_eval(self._eval_step())
 
-        Warning: If the evaluation value isn't cached, it will be evaluated
-        recursively.
-
-        """
+    def _eval_step(self):
         if len(self._tuple) == 0:
-            raise InvalidExpression()
+            raise InvalidExpression("Empty expression.")
 
         if self._eval_obj is not self.null:
-            return self._eval_obj
+            yield self._eval_obj
         else:
             op = self._tuple[0]
-            op = getattr(op, "eval_obj", op)
+
+            if isinstance(op, (ExpressionTuple, KwdPair)):
+                op = yield op._eval_step()
 
             if not callable(op):
-                raise InvalidExpression()
+                raise InvalidExpression(
+                    "ExpressionTuple does not have a callable operator."
+                )
 
-            evaled_args = [getattr(i, "eval_obj", i) for i in self._tuple[1:]]
-            arg_grps = toolz.groupby(lambda x: isinstance(x, KwdPair), evaled_args)
-            evaled_args = arg_grps.get(False, [])
-            evaled_kwargs = arg_grps.get(True, [])
+            evaled_args = []
+            evaled_kwargs = []
+            for i in self._tuple[1:]:
+                if isinstance(i, (ExpressionTuple, KwdPair)):
+                    i = yield i._eval_step()
+
+                if isinstance(i, KwdPair):
+                    evaled_kwargs.append(i)
+                else:
+                    evaled_args.append(i)
 
             try:
                 op_sig = inspect.signature(op)
@@ -150,7 +196,7 @@ class ExpressionTuple(Sequence):
             # assert not isinstance(_eval_obj, ExpressionTuple)
 
             self._eval_obj = _eval_obj
-            return self._eval_obj
+            yield self._eval_obj
 
     @eval_obj.setter
     def eval_obj(self, obj):
@@ -221,9 +267,32 @@ class ExpressionTuple(Sequence):
                     p.pretty(item)
 
     def __eq__(self, other):
-        return self._tuple == other
+
+        # Built-in `==` won't work in CPython for deeply nested structures.
+
+        # TODO: We could track the level of `ExpressionTuple`-only nesting and
+        # apply TCO only when it reaches a certain level.
+
+        if not isinstance(other, Sequence):
+            return NotImplemented
+
+        if len(other) != len(self):
+            return False
+
+        queue = deque(zip(self._tuple, other))
+
+        while queue:
+            i_s, i_o = queue.pop()
+
+            if isinstance(i_s, ExpressionTuple) or isinstance(i_o, ExpressionTuple):
+                queue.extend(zip(i_s, i_o))
+            elif i_s != i_o:
+                return False
+
+        return True
 
     def __hash__(self):
+        # XXX: CPython fails for deeply nested tuples!
         return hash(self._tuple)
 
 
diff --git a/etuples/dispatch.py b/etuples/dispatch.py
index d8fa06e..b9bb262 100644
--- a/etuples/dispatch.py
+++ b/etuples/dispatch.py
@@ -4,7 +4,7 @@ from multipledispatch import dispatch
 
 from cons.core import ConsError, ConsNull, ConsPair, car, cdr, cons
 
-from .core import etuple, ExpressionTuple
+from .core import etuple, ExpressionTuple, trampoline_eval
 
 try:
     from packaging import version
@@ -103,30 +103,46 @@ def etuplize(x, shallow=False, return_bad_args=False, convert_ConsPairs=True):
       of raising an exception.
 
     """
-    if isinstance(x, ExpressionTuple):
-        return x
-    elif convert_ConsPairs and x is not None and isinstance(x, (ConsNull, ConsPair)):
-        return etuple(*x)
-
-    try:
-        op, args = rator(x), rands(x)
-    except ConsError:
-        op, args = None, None
-
-    if not callable(op) or not isinstance(args, (ConsNull, ConsPair)):
-        if return_bad_args:
-            return x
+
+    def etuplize_step(
+        x,
+        shallow=shallow,
+        return_bad_args=return_bad_args,
+        convert_ConsPairs=convert_ConsPairs,
+    ):
+        if isinstance(x, ExpressionTuple):
+            yield x
+            return
+        elif (
+            convert_ConsPairs and x is not None and isinstance(x, (ConsNull, ConsPair))
+        ):
+            yield etuple(*x)
+            return
+
+        try:
+            op, args = rator(x), rands(x)
+        except ConsError:
+            op, args = None, None
+
+        if not callable(op) or not isinstance(args, (ConsNull, ConsPair)):
+            if return_bad_args:
+                yield x
+                return
+            else:
+                raise TypeError(f"x is neither a non-str Sequence nor term: {type(x)}")
+
+        if shallow:
+            et_op = op
+            et_args = args
         else:
-            raise TypeError(f"x is neither a non-str Sequence nor term: {type(x)}")
-
-    if shallow:
-        et_op = op
-        et_args = args
-    else:
-        et_op = etuplize(op, return_bad_args=True)
-        et_args = tuple(
-            etuplize(a, return_bad_args=True, convert_ConsPairs=False) for a in args
-        )
+            et_op = yield etuplize_step(op, return_bad_args=True)
+            et_args = []
+            for a in args:
+                e = yield etuplize_step(
+                    a, return_bad_args=True, convert_ConsPairs=False
+                )
+                et_args.append(e)
+
+        yield etuple(et_op, *et_args, eval_obj=x)
 
-    res = etuple(et_op, *et_args, eval_obj=x)
-    return res
+    return trampoline_eval(etuplize_step(x))
diff --git a/setup.py b/setup.py
index 437f2da..3d0e3e0 100644
--- a/setup.py
+++ b/setup.py
@@ -13,7 +13,7 @@ setup(
     maintainer="Brandon T. Willard",
     maintainer_email="brandonwillard+etuples@gmail.com",
     packages=["etuples"],
-    install_requires=["toolz", "cons", "multipledispatch",],
+    install_requires=["cons", "multipledispatch",],
     long_description=open("README.md").read() if exists("README.md") else "",
     long_description_content_type="text/markdown",
     python_requires=">=3.6",


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 9e106b7..e1a2577 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -1,3 +1,5 @@
+import sys
+
 import pytest
 
 from operator import add
@@ -11,6 +13,12 @@ def test_ExpressionTuple(capsys):
     assert hash(e0) == hash((add, 1, 2))
     assert e0 == ExpressionTuple(e0)
 
+    e5 = ExpressionTuple((1, ExpressionTuple((2, 3))))
+    e6 = ExpressionTuple((1, ExpressionTuple((2, 3))))
+
+    assert e5 == e6
+    assert hash(e5) == hash(e6)
+
     # Not sure if we really want this; it's more
     # common to have a copy constructor, no?
     assert e0 is ExpressionTuple(e0)
@@ -51,6 +59,10 @@ def test_ExpressionTuple(capsys):
     with pytest.raises(InvalidExpression):
         e4.eval_obj
 
+    assert ExpressionTuple((ExpressionTuple((lambda: add,)), 1, 1)).eval_obj == 2
+    assert ExpressionTuple((1, 2)) != ExpressionTuple((1,))
+    assert ExpressionTuple((1, 2)) != ExpressionTuple((1, 3))
+
 
 def test_etuple():
     """Test basic `etuple` functionality."""
@@ -183,3 +195,48 @@ def test_pprint():
         pretty_mod.pretty(et)
         == "e(\n  1,\n  e('a', 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19),\n  e(3, 'b'),\n  blah=e(c, 0))"
     )
+
+
+def gen_long_add_chain(N=None, num=1):
+    b_struct = num
+    if N is None:
+        N = sys.getrecursionlimit()
+    for i in range(0, N):
+        b_struct = etuple(add, num, b_struct)
+    return b_struct
+
+
+def test_reify_recursion_limit():
+
+    a = gen_long_add_chain(10)
+    assert a.eval_obj == 11
+
+    r_limit = sys.getrecursionlimit()
+
+    try:
+        sys.setrecursionlimit(100)
+
+        a = gen_long_add_chain(200)
+        assert a.eval_obj == 201
+
+        b = gen_long_add_chain(200, num=2)
+        assert b.eval_obj == 402
+
+        c = gen_long_add_chain(200)
+        assert a == c
+
+    finally:
+        sys.setrecursionlimit(r_limit)
+
+
+@pytest.mark.xfail(strict=True)
+def test_reify_recursion_limit_hash():
+    r_limit = sys.getrecursionlimit()
+
+    try:
+        sys.setrecursionlimit(100)
+        a = gen_long_add_chain(200)
+        # CPython uses the call stack and fails
+        assert hash(a)
+    finally:
+        sys.setrecursionlimit(r_limit)