Rule sort refactor

mathics/builtin/arithfns/basic.py

@@ -62,7 +62,7 @@ class CubeRoot(Builtin):
     }
 
     rules = {
-        "CubeRoot[n_?NumberQ]": "If[n > 0, Power[n, Divide[1, 3]], Times[-1, Power[Times[-1, n], Divide[1, 3]]]]",
+        "CubeRoot[n_Integer|n_Real|n_Rational]": "If[n > 0, Power[n, Divide[1, 3]], Times[-1, Power[Times[-1, n], Divide[1, 3]]]]",
         "CubeRoot[n_]": "Power[n, Divide[1, 3]]",
         "MakeBoxes[CubeRoot[x_], f:StandardForm|TraditionalForm]": (
             "RadicalBox[MakeBoxes[x, f], 3]"

mathics/builtin/symbol/properties.py

@@ -194,7 +194,7 @@ class DownValues(Builtin):
     >> f[x_Integer] := 2
     >> f[x_Real] := 3
     >> DownValues[f]
-     = {HoldPattern[f[x_Real]] ⧴ 3, HoldPattern[f[x_Integer]] ⧴ 2, HoldPattern[f[x_]] ⧴ x ^ 2}
+     = {HoldPattern[f[x_Integer]] ⧴ 2, HoldPattern[f[x_Real]] ⧴ 3, HoldPattern[f[x_]] ⧴ x ^ 2}
     >> f[3]
      = 2
     >> f[3.]

mathics/core/builtin.py

@@ -309,19 +309,6 @@ def contribute(self, definitions: Definitions, is_pymodule=False):
             PyMathicsDefinitions() if is_pymodule else SystemDefinitions()
         )
 
-        for pattern, function in self.get_functions(
-            prefix="eval", is_pymodule=is_pymodule
-        ):
-            pat_attr = attributes if pattern.get_head_name() == name else None
-            rules.append(
-                FunctionApplyRule(
-                    name,
-                    pattern,
-                    function,
-                    check_options,
-                    attributes=pat_attr,
-                )
-            )
         for pattern_str, replace_str in self.rules.items():
             pattern_str = pattern_str % {"name": name}
             pattern = parse_builtin_rule(pattern_str, definition_class)
@@ -335,6 +322,19 @@ def contribute(self, definitions: Definitions, is_pymodule=False):
                 )
             )
 
+        for pattern, function in self.get_functions(
+            prefix="eval", is_pymodule=is_pymodule
+        ):
+            pat_attr = attributes if pattern.get_head_name() == name else None
+            rules.append(
+                FunctionApplyRule(
+                    name,
+                    pattern,
+                    function,
+                    check_options,
+                    attributes=pat_attr,
+                )
+            )
         box_rules = []
         # FIXME: Why a special case for System`MakeBoxes? Remove this
         if name != "System`MakeBoxes":
@@ -371,17 +371,6 @@ def contextify_form_name(f):
             return forms, pattern
 
         formatvalues: dict[str, List[BaseRule]] = {"": []}
-        for pattern, function in self.get_functions("format_"):
-            forms, pattern = extract_forms(pattern)
-            pat_attr = attributes if pattern.get_head_name() == name else None
-            for form in forms:
-                if form not in formatvalues:
-                    formatvalues[form] = []
-                formatvalues[form].append(
-                    FunctionApplyRule(
-                        name, pattern, function, None, attributes=pat_attr
-                    )
-                )
         for pattern, replace in self.formats.items():
             forms, pattern = extract_forms(pattern)
             for form in forms:
@@ -395,6 +384,18 @@ def contextify_form_name(f):
                     RewriteRule(pattern, parse_builtin_rule(replace))
                 )
 
+        for pattern, function in self.get_functions("format_"):
+            forms, pattern = extract_forms(pattern)
+            pat_attr = attributes if pattern.get_head_name() == name else None
+            for form in forms:
+                if form not in formatvalues:
+                    formatvalues[form] = []
+                formatvalues[form].append(
+                    FunctionApplyRule(
+                        name, pattern, function, None, attributes=pat_attr
+                    )
+                )
+
         formatvalues.setdefault("_MakeBoxes", []).extend(box_rules)
 
         for form, formatrules in formatvalues.items():

mathics/core/definitions.py

@@ -5,7 +5,6 @@
 """
 
 import base64
-import bisect
 import os.path as osp
 import pickle
 import re
@@ -19,6 +18,7 @@
 from mathics.core.convert.expression import to_mathics_list
 from mathics.core.element import BaseElement, fully_qualified_symbol_name
 from mathics.core.pattern import BasePattern
+from mathics.core.pattern.specificity import rule_le
 from mathics.core.rules import BaseRule, RewriteRule
 from mathics.core.symbols import Atom, Symbol, SymbolList, strip_context
 from mathics.core.systemsymbols import (
@@ -1013,9 +1013,35 @@ def is_conditional(x) -> bool:
                 del values[index]
                 break
 
-    # use insort_left to guarantee that if equal rules exist, newer rules will
-    # get higher precedence by being inserted before them. see DownValues[].
-    bisect.insort_left(values, rule, key=lambda x: x.pattern_precedence)
+    # Mathematica orders rules with a partial order: a rule goes before
+    # another one only if it is strictly more specific. Rules which are
+    # not comparable keep their definition order. `insort_right` only
+    # uses `<`, so a key wrapper with a partial `__lt__` is enough.
+    # When `rule` is equivalent to an existing rule, it is placed before it,
+    # so newer rules get higher precedence. see DownValues[].
+    # Insert before the first existing rule which strictly contains the new
+    # one. A rule which is not comparable with the others keeps its
+    # definition order, as in Mathematica. A linear scan is used because
+    # the relation is partial, so a binary search is not reliable.
+    i = 0
+    for existing in values:
+        if _rule_lt(rule, existing):
+            break
+        i += 1
+    values.insert(i, rule)
+
+
+def _rule_lt(new: BaseRule, existing: BaseRule) -> bool:
+    """True if `new` should go before `existing`: `new` matches a subset of
+    what `existing` matches (or they are equivalent).
+
+    If the patterns involved are not understood by
+    `mathics.core.pattern.specificity`, fall back to the sort keys.
+    """
+    contained = rule_le(new, existing)
+    if contained is None:
+        return new.pattern_precedence <= existing.pattern_precedence
+    return contained
 
 
 def merge_definitions(candidates: List[Definition]) -> Definition: