Improving precedence comparison. (#1154)

This PR includes some fixes and improvings for handling operator
precedence. In particular, as in WMA, avoids wrapping in parenthesis the
second element of `Rule[pat_, Function[...]]`.

mathics/builtin/atomic/symbols.py

@@ -527,7 +527,7 @@ def lhs(expr):
                         return Expression(SymbolFormat, expr, Symbol(format))
 
                     def rhs(expr):
-                        if expr.has_formf(SymbolInfix, None):
+                        if expr.has_form(SymbolInfix, None):
                             expr = Expression(
                                 Expression(SymbolHoldForm, expr.head), *expr.elements
                             )

mathics/builtin/layout.py

@@ -16,7 +16,7 @@
 from mathics.builtin.makeboxes import MakeBoxes
 from mathics.builtin.options import options_to_rules
 from mathics.core.atoms import Real, String
-from mathics.core.builtin import BinaryOperator, Builtin, Operator
+from mathics.core.builtin import Builtin, Operator, PostfixOperator, PrefixOperator
 from mathics.core.expression import Evaluation, Expression
 from mathics.core.list import ListExpression
 from mathics.core.symbols import Symbol
@@ -214,7 +214,7 @@ class NonAssociative(Builtin):
     summary_text = "non-associative operator"
 
 
-class Postfix(BinaryOperator):
+class Postfix(PostfixOperator):
     """
     <url>:WMA link:https://reference.wolfram.com/language/ref/Postfix.html</url>
 
@@ -294,7 +294,7 @@ class PrecedenceForm(Builtin):
     summary_text = "parenthesize with a precedence"
 
 
-class Prefix(BinaryOperator):
+class Prefix(PrefixOperator):
     """
     <url>:WMA link:https://reference.wolfram.com/language/ref/Prefix.html</url>
 

mathics/builtin/numbers/diffeqns.py

@@ -31,11 +31,11 @@ class DSolve(Builtin):
      = {{y[x] -> C[1] E ^ (-x) + C[2] E ^ x}}
 
     >> DSolve[y''[x] == y[x], y, x]
-     = {{y -> (Function[{x}, C[1] E ^ (-x) + C[2] E ^ x])}}
+     = {{y -> Function[{x}, C[1] E ^ (-x) + C[2] E ^ x]}}
 
     DSolve can also solve basic PDE
     >> DSolve[D[f[x, y], x] / f[x, y] + 3 D[f[x, y], y] / f[x, y] == 2, f, {x, y}]
-     = {{f -> (Function[{x, y}, E ^ (x / 5 + 3 y / 5) C[1][3 x - y]])}}
+     = {{f -> Function[{x, y}, E ^ (x / 5 + 3 y / 5) C[1][3 x - y]]}}
 
     >> DSolve[D[f[x, y], x] x + D[f[x, y], y] y == 2, f[x, y], {x, y}]
      = {{f[x, y] -> 2 Log[x] + C[1][y / x]}}

mathics/builtin/patterns/rules.py

@@ -503,11 +503,11 @@ class Rule_(BinaryOperator):
      = a
     """
 
-    name = "Rule"
-    operator = "->"
     attributes = A_SEQUENCE_HOLD | A_PROTECTED
     grouping = "Right"
+    name = "Rule"
     needs_verbatim = True
+    operator = "->"
     summary_text = "a replacement rule"
 
     def eval_rule(self, elems, evaluation):

mathics/builtin/recurrence.py

@@ -40,18 +40,18 @@ class RSolve(Builtin):
 
     No boundary conditions gives two general parameters:
     >> RSolve[{a[n + 2] == a[n]}, a, n]
-     = {{a -> (Function[{n}, C[0] + C[1] (-1) ^ n])}}
+     = {{a -> Function[{n}, C[0] + C[1] (-1) ^ n]}}
 
     Include one boundary condition:
     >> RSolve[{a[n + 2] == a[n], a[0] == 1}, a, n]
      = ...
     ## Order of terms depends on interpreter:
-    ## PyPy:    {{a -> (Function[{n}, 1 - C[1] + C[1] -1 ^ n])}}
-    ## CPython: {{a -> (Function[{n}, 1 + C[1] -1 ^ n - C[1]])}
+    ## PyPy:    {{a -> Function[{n}, 1 - C[1] + C[1] -1 ^ n]}}
+    ## CPython: {{a -> Function[{n}, 1 + C[1] -1 ^ n - C[1]]}
 
-    Geta "pure function" solution for a with two boundary conditions:
+    Get a "pure function" solution for a with two boundary conditions:
     >> RSolve[{a[n + 2] == a[n], a[0] == 1, a[1] == 4}, a, n]
-     = {{a -> (Function[{n}, 5 / 2 - 3 (-1) ^ n / 2])}}
+     = {{a -> Function[{n}, 5 / 2 - 3 (-1) ^ n / 2]}}
     """
 
     messages = {

mathics/core/builtin.py

@@ -1151,22 +1151,29 @@ def __init__(self, *args, **kwargs):
 
         # Prevent pattern matching symbols from gaining meaning here using
         # Verbatim
-        name = f"Verbatim[{name}]"
+        verbatim_name = f"Verbatim[{name}]"
 
         # For compatibility, allow grouping symbols in builtins to be
         # specified without System`.
         self.grouping = ensure_context(self.grouping)
 
         if self.grouping in ("System`None", "System`NonAssociative"):
-            op_pattern = f"{name}[items__]"
+            op_pattern = f"{verbatim_name}[items__]"
             replace_items = "items"
         else:
-            op_pattern = f"{name}[x_, y_]"
+            op_pattern = f"{verbatim_name}[x_, y_]"
             replace_items = "x, y"
 
         operator = ascii_operator_to_symbol.get(self.operator, self.__class__.__name__)
 
         if self.default_formats:
+            if name not in ("Rule", "RuleDelayed"):
+                formats = {
+                    op_pattern: "HoldForm[Infix[{%s}, %s, %d, %s]]"
+                    % (replace_items, operator, self.precedence, self.grouping)
+                }
+                formats.update(self.formats)
+                self.formats = formats
             formatted = "MakeBoxes[Infix[{%s}, %s, %d,%s], form]" % (
                 replace_items,
                 operator,

mathics/eval/makeboxes.py

@@ -6,8 +6,7 @@
 """
 
 
-import typing
-from typing import Any, Callable, Dict, Optional, Type
+from typing import Any, Callable, Dict, List, Optional, Type
 
 from mathics.core.atoms import Complex, Integer, Rational, Real, String, SymbolI
 from mathics.core.convert.expression import to_expression_with_specialization
@@ -70,6 +69,35 @@ def _boxed_string(string: str, **options):
     return StyleBox(String(string), **options)
 
 
+def compare_precedence(
+    element: BaseElement, precedence: Optional[int] = None
+) -> Optional[int]:
+    """
+    compare the precedence of the element regarding a precedence value.
+    If both precedences are equal, return 0. If precedence of the
+    first element is higher, return 1, otherwise -1.
+    If precedences cannot be compared, return None.
+    """
+    while element.has_form("HoldForm", 1):
+        element = element.elements[0]
+
+    if precedence is None:
+        return None
+    if element.has_form(("Infix", "Prefix", "Postfix"), 3, None):
+        element_prec = element.elements[2].value
+    elif element.has_form("PrecedenceForm", 2):
+        element_prec = element.elements[1].value
+    # For negative values, ensure that the element_precedence is at least the precedence. (Fixes #332)
+    elif isinstance(element, (Integer, Real)) and element.value < 0:
+        element_prec = precedence
+    else:
+        element_prec = builtins_precedence.get(element.get_head_name())
+
+    if element_prec is None:
+        return None
+    return 0 if element_prec == precedence else (1 if element_prec > precedence else -1)
+
+
 # 640 = sys.int_info.str_digits_check_threshold.
 # Someday when 3.11 is the minimum version of Python supported,
 # we can replace the magic value 640 below with sys.int.str_digits_check_threshold.
@@ -211,7 +239,6 @@ def do_format_element(
     Applies formats associated to the expression and removes
     superfluous enclosing formats.
     """
-
     from mathics.core.definitions import OutputForms
 
     evaluation.inc_recursion_depth()
@@ -234,6 +261,7 @@ def do_format_element(
             if include_form:
                 expr = Expression(form, expr)
             return expr
+
         # Repeated and RepeatedNull confuse the formatter,
         # so we need to hardlink their format rules:
         if head is SymbolRepeated:
@@ -279,8 +307,8 @@ def format_expr(expr):
 
         formatted = format_expr(expr) if isinstance(expr, EvalMixin) else None
         if formatted is not None:
-            do_format = element_formatters.get(type(formatted), do_format_element)
-            result = do_format(formatted, evaluation, form)
+            do_format_fn = element_formatters.get(type(formatted), do_format_element)
+            result = do_format_fn(formatted, evaluation, form)
             if include_form and result is not None:
                 result = Expression(form, result)
             return result
@@ -297,8 +325,8 @@ def format_expr(expr):
             # just return it as it is.
             if len(expr.get_elements()) != 1:
                 return expr
-            do_format = element_formatters.get(type(element), do_format_element)
-            result = do_format(expr, evaluation, form)
+            do_format_fn = element_formatters.get(type(element), do_format_element)
+            result = do_format_fn(expr, evaluation, form)
             if isinstance(result, Expression):
                 expr = result
 
@@ -307,13 +335,14 @@ def format_expr(expr):
             and not isinstance(expr, (Atom, BoxElementMixin))
             and head not in (SymbolGraphics, SymbolGraphics3D)
         ):
-            # print("Not inside graphics or numberform, and not is atom")
-            new_elements = [
-                element_formatters.get(type(element), do_format_element)(
-                    element, evaluation, form
+            new_elements = tuple(
+                (
+                    element_formatters.get(type(element), do_format_element)(
+                        element, evaluation, form
+                    )
+                    for element in expr.elements
                 )
-                for element in expr.elements
-            ]
+            )
             expr_head = expr.head
             do_format = element_formatters.get(type(expr_head), do_format_element)
             head = do_format(expr_head, evaluation, form)
@@ -367,7 +396,7 @@ def do_format_complex(
             form,
         )
 
-    parts: typing.List[Any] = []
+    parts: List[Any] = []
     if element.is_machine_precision() or not element.real.is_zero:
         parts.append(element.real)
     if element.imag.sameQ(Integer(1)):
@@ -418,27 +447,12 @@ def parenthesize(
     If when_equal is True, parentheses will be added if the two
     precedence values are equal.
     """
-    while element.has_form("HoldForm", 1):
-        element = element.elements[0]
-
-    if element.has_form(("Infix", "Prefix", "Postfix"), 3, None):
-        element_prec = element.elements[2].value
-    elif element.has_form("PrecedenceForm", 2):
-        element_prec = element.elements[1].value
-    # If "element" is a negative number, we need to parenthesize the number. (Fixes #332)
-    elif isinstance(element, (Integer, Real)) and element.value < 0:
-        # Force parenthesis by adjusting the surrounding context's precedence value,
-        # We can't change the precedence for the number since it, doesn't
-        # have a precedence value.
-        element_prec = precedence
-    else:
-        element_prec = builtins_precedence.get(element.get_head())
-    if precedence is not None and element_prec is not None:
-        if precedence > element_prec or (precedence == element_prec and when_equal):
-            return Expression(
-                SymbolRowBox,
-                ListExpression(StringLParen, element_boxes, StringRParen),
-            )
+    cmp = compare_precedence(element, precedence)
+    if cmp is not None and (cmp == -1 or cmp == 0 and when_equal):
+        return Expression(
+            SymbolRowBox,
+            ListExpression(String("("), element_boxes, String(")")),
+        )
     return element_boxes
 
 

test/builtin/numbers/test_diffeqns.py

@@ -45,19 +45,19 @@
         (
             "DSolve[f'[x] == f[x], f, x] /. {C[1] -> 1}",
             None,
-            "{{f -> (Function[{x}, 1 E ^ x])}}",
+            "{{f -> Function[{x}, 1 E ^ x]}}",
             None,
         ),
         (
             "DSolve[f'[x] == f[x], f, x] /. {C -> D}",
             None,
-            "{{f -> (Function[{x}, D[1] E ^ x])}}",
+            "{{f -> Function[{x}, D[1] E ^ x]}}",
             None,
         ),
         (
             "DSolve[f'[x] == f[x], f, x] /. {C[1] -> C[0]}",
             None,
-            "{{f -> (Function[{x}, C[0] E ^ x])}}",
+            "{{f -> Function[{x}, C[0] E ^ x]}}",
             None,
         ),
         (
@@ -66,11 +66,11 @@
             "DSolve[f[x] == 0, f, {}]",
             None,
         ),
-        ## Order of arguments shoudn't matter
+        # # Order of arguments shoudn't matter
         (
             "DSolve[D[f[x, y], x] == D[f[x, y], y], f, {x, y}]",
             None,
-            "{{f -> (Function[{x, y}, C[1][-x - y]])}}",
+            "{{f -> Function[{x, y}, C[1][-x - y]]}}",
             None,
         ),
         (
@@ -88,7 +88,7 @@
         (
             "DSolve[\\[Gamma]'[x] == 0, \\[Gamma], x]",
             None,
-            "{{γ -> (Function[{x}, C[1]])}}",
+            "{{γ -> Function[{x}, C[1]]}}",
             "sympy #11669 test",
         ),
     ],