Build expression trees in PointerSymbol

This enables correct handling of precedence, e.g.

s = sym.A + sym.B
s * sym.C

will be correctly handled as (sym.A + sym.B) * sym.C instead of
sym.A + (sym.B * sym.C).

Fixes #267.

CHANGES.rst

@@ -23,6 +23,12 @@ Release History
 1.1.1 (unreleased)
 ==================
 
+**Fixed**
+
+- ``PointerSymbol`` precedence handling has been fixed. For example,
+  ``(sym.A + sym.B) * sym.C`` will be evaluated correctly now.
+  (`#267 <https://github.com/nengo/nengo_spa/issues/267>`__,
+  `#268 <https://github.com/nengo/nengo_spa/pull/268>`__)
 
 
 

nengo_spa/ast/symbolic.py

@@ -5,6 +5,7 @@
 import nengo
 import numpy as np
 
+from nengo_spa.ast import expr_tree
 from nengo_spa.ast.base import infer_types, Fixed, TypeCheckedBinaryOp
 from nengo_spa.exceptions import SpaTypeError
 from nengo_spa.semantic_pointer import SemanticPointer
@@ -51,14 +52,20 @@ def evaluate(self):
     def expr(self):
         return repr(self.value)
 
+    @property
+    def _expr_tree(self):
+        return expr_tree.Leaf(self.expr)
+
     def __neg__(self):
         return FixedScalar(-self.value)
 
 
 class PointerSymbol(Symbol):
     def __init__(self, expr, type_=TAnyVocab):
         super(PointerSymbol, self).__init__(type_=type_)
-        self._expr = expr
+        self._expr_tree = (
+            expr if isinstance(expr, expr_tree.Node) else expr_tree.Leaf(expr)
+        )
 
     def connect_to(self, sink, **kwargs):
         return nengo.Connection(self.construct(), sink, **kwargs)
@@ -76,20 +83,20 @@ def evaluate(self):
 
     @property
     def expr(self):
-        return self._expr
+        return str(self._expr_tree)
 
     def __invert__(self):
-        return PointerSymbol("~" + self.expr, self.type)
+        return PointerSymbol(~self._expr_tree, self.type)
 
     def __neg__(self):
-        return PointerSymbol("-" + self.expr, self.type)
+        return PointerSymbol(-self._expr_tree, self.type)
 
     def __add__(self, other):
         other = as_symbolic_node(other)
         if not isinstance(other, PointerSymbol):
             return NotImplemented
         type_ = infer_types(self, other)
-        return PointerSymbol(self.expr + "+" + other.expr, type_)
+        return PointerSymbol(self._expr_tree + other._expr_tree, type_)
 
     def __radd__(self, other):
         return self + other
@@ -99,25 +106,25 @@ def __sub__(self, other):
         if not isinstance(other, PointerSymbol):
             return NotImplemented
         type_ = infer_types(self, other)
-        return PointerSymbol(self.expr + "-" + other.expr, type_)
+        return PointerSymbol(self._expr_tree - other._expr_tree, type_)
 
     def __rsub__(self, other):
         return (-self) + other
 
     @symbolic_op
     def __mul__(self, other):
         type_ = infer_types(self, other)
-        return PointerSymbol(self.expr + "*" + other.expr, type_)
+        return PointerSymbol(self._expr_tree * other._expr_tree, type_)
 
     @symbolic_op
     def __rmul__(self, other):
         type_ = infer_types(self, other)
-        return PointerSymbol(other.expr + "*" + self.expr, type_)
+        return PointerSymbol(other._expr_tree * self._expr_tree, type_)
 
     @symbolic_op
     def __truediv__(self, other):
         type_ = infer_types(self, other)
-        return PointerSymbol(self.expr + "/" + other.expr, type_)
+        return PointerSymbol(self._expr_tree / other._expr_tree, type_)
 
     def dot(self, other):
         other = as_symbolic_node(other)
@@ -145,7 +152,7 @@ def translate(self, vocab, populate=None, keys=None, solver=None):
         return SemanticPointer(np.dot(tr, self.evaluate().v), vocab=vocab)
 
     def __repr__(self):
-        return "PointerSymbol({!r}, {!r})".format(self.expr, self.type)
+        return "PointerSymbol({!r}, {!r})".format(self._expr_tree, self.type)
 
 
 class PointerSymbolFactory:
@@ -165,7 +172,7 @@ def __getattribute__(self, key):
         return PointerSymbol(key)
 
     def __call__(self, expr):
-        return PointerSymbol(re.sub(r"\s+", "", expr))
+        return PointerSymbol("({})".format(re.sub(r"\s+", "", expr)))
 
 
 sym = PointerSymbolFactory()

nengo_spa/ast/tests/test_expr_tree.py

@@ -37,5 +37,6 @@ def test_unary_operator(op):
 @pytest.mark.parametrize("op", ["<<", ">>", "+", "-", "*", "@", "/", "//", "%", "**"])
 def test_binary_operator(op):
     lhs, rhs = Leaf("lhs"), Leaf("rhs")
+    assert lhs
+    assert rhs
     assert str(eval("lhs" + op + "rhs")) == "lhs {} rhs".format(op)
-

nengo_spa/ast/tests/test_symbolic.py

@@ -153,7 +153,15 @@ def test_pointer_symbol_factory():
     assert ps.expr == "A"
 
 
-def test_pointer_symbol_factory_expressions():
-    ps = sym("A + B * C")
+@pytest.mark.parametrize(
+    "ps,expected",
+    [
+        (sym("A + B * C"), "(A+B*C)"),
+        (sym.A + sym.B * sym.C, "A + B * C"),
+        (sym("(A + B) * C"), "((A+B)*C)"),
+        ((sym.A + sym.B) * sym.C, "(A + B) * C"),
+    ],
+)
+def test_pointer_symbol_factory_expressions(ps, expected):
     assert isinstance(ps, PointerSymbol)
-    assert ps.expr == (sym.A + sym.B * sym.C).expr
+    assert ps.expr == expected