apply_rule -> apply_function ... (#1084)

Change `apply_rule()` -> to `apply_function()` when that is what is is;
(and not when it is not).

Also mark BaseRule an abstract class, and go over docstrings in
mathics.core.rules

@mmatera I think i now understand what you were getting at when we were
discussion replacement name for `do_replace()`. Both functions are
definitely apply functions. We could call the method `apply()`. I think
though being specific and explicit increases clarity and reduces
potential confusion.

mathics/builtin/trace.py

@@ -33,7 +33,9 @@
 from mathics.core.symbols import SymbolFalse, SymbolNull, SymbolTrue, strip_context
 
 
-def traced_apply_rule(self, expression, vars, options: dict, evaluation: Evaluation):
+def traced_apply_function(
+    self, expression, vars, options: dict, evaluation: Evaluation
+):
     if options and self.check_options:
         if not self.check_options(options, evaluation):
             return None
@@ -185,7 +187,7 @@ class TraceBuiltins(_TraceBase):
     """
 
     definitions_copy: Definitions
-    apply_rule_copy: Callable
+    apply_function_copy: Callable
 
     function_stats: "defaultdict" = defaultdict(
         lambda: {"count": 0, "elapsed_milliseconds": 0.0}
@@ -238,19 +240,19 @@ def sort_by_name(tup: tuple):
     @staticmethod
     def enable_trace(evaluation) -> None:
         if TraceBuiltins.traced_definitions is None:
-            TraceBuiltins.apply_rule_copy = BuiltinRule.apply_rule
+            TraceBuiltins.apply_function_copy = BuiltinRule.apply_function
             TraceBuiltins.definitions_copy = evaluation.definitions
 
-            # Replaces apply_rule by the custom one
-            BuiltinRule.apply_rule = traced_apply_rule
-            # Create new definitions uses the new apply_rule
+            # Replaces apply_function by the custom one
+            BuiltinRule.apply_function = traced_apply_function
+            # Create new definitions uses the new apply_function
             evaluation.definitions = Definitions(add_builtin=True)
         else:
             evaluation.definitions = TraceBuiltins.definitions_copy
 
     @staticmethod
     def disable_trace(evaluation) -> None:
-        BuiltinRule.apply_rule = TraceBuiltins.apply_rule_copy
+        BuiltinRule.apply_function = TraceBuiltins.apply_function_copy
         evaluation.definitions = TraceBuiltins.definitions_copy
 
     def eval(self, expr, evaluation, options={}):

mathics/core/rules.py

@@ -1,6 +1,15 @@
-# cython: language_level=3
 # -*- coding: utf-8 -*-
+"""Rules are a core part of the way WMA and Mathics3 executes a
+program.  Expressions can be transformed by rewrite rules (AKA
+transformation rules); builtin functions get matched and applied via a
+function signature specified using a BuiltinRule.
 
+This module contains the classes for these two types of rules.
+
+"""
+
+
+from abc import ABC
 from inspect import signature
 from itertools import chain
 from typing import Callable, Optional
@@ -21,21 +30,24 @@ def function_arguments(f):
 
 
 class StopGenerator_BaseRule(StopGenerator):
+    """
+    Signals that there are no more rules to check for pattern matching
+    """
+
     pass
 
 
-class BaseRule(KeyComparable):
+class BaseRule(KeyComparable, ABC):
     """
-    This is the base class from which all other Rules are derived from.
+    This is the base class from which BuiltinRule and Rule classes
+    are derived from.
 
-    Rules are part of the rewriting system of Mathics. See
+    Rules are part of the rewriting system of Mathics3. See
     https://en.wikipedia.org/wiki/Rewriting
 
     This class is not complete in of itself and subclasses should
-    adapt or fill in what is needed. In particular ``apply_rule()``
-    needs to be implemented.
-
-    Important subclasses: BuiltinRule and Rule.
+    adapt or fill in what is needed. In particular either ``apply_rule()``
+    or ``apply_function()`` need to be implemented.
 
     Note: we want Rules to be serializable so that we can dump and
     restore Rules in order to make startup time faster.
@@ -75,7 +87,12 @@ def yield_match(vars, rest):
                 if name.startswith("_option_"):
                     options[name[len("_option_") :]] = value
                     del vars[name]
-            new_expression = self.apply_rule(expression, vars, options, evaluation)
+            apply_fn = (
+                self.apply_function
+                if isinstance(self, BuiltinRule)
+                else self.apply_rule
+            )
+            new_expression = apply_fn(expression, vars, options, evaluation)
             if new_expression is None:
                 new_expression = expression
             if rest[0] or rest[1]:
@@ -123,17 +140,22 @@ def yield_match(vars, rest):
     def apply_rule(self):
         raise NotImplementedError
 
+    def apply_function(self):
+        raise NotImplementedError
+
     def get_sort_key(self) -> tuple:
         # FIXME: check if this makes sense:
         return tuple((self.system, self.pattern.get_sort_key(True)))
 
 
+# FIXME: the class name would be better called RewiteRule.
 class Rule(BaseRule):
-    """There are two kinds of Rules.  This kind of Rule transforms an
-    Expression into another Expression based on the pattern and a
-    replacement term and doesn't involve function application.
+    """There are two kinds of Rules.  This kind of is a rewrite rule
+    and transforms an Expression into another Expression based on the
+    pattern and a replacement term and doesn't involve function
+    application.
 
-    Also, in contrast to BuiltinRule[], rule application cannot force
+    In contrast to BuiltinRule[], rule application cannot force
     a reevaluation of the expression when the rewrite/apply/eval step
     finishes.
 
@@ -148,6 +170,7 @@ class Rule(BaseRule):
 
     Note: we want Rules to be serializable so that we can dump and
     restore Rules in order to make startup time faster.
+
     """
 
     def __init__(
@@ -193,6 +216,7 @@ def __repr__(self) -> str:
         return "<Rule: %s -> %s>" % (self.pattern, self.replace)
 
 
+# FIXME: the class name would be better called FunctionCallRule.
 class BuiltinRule(BaseRule):
     """
     A BuiltinRule is a rule that has a replacement term that is associated
@@ -249,9 +273,9 @@ def __init__(
         self.check_options = check_options
         self.pass_expression = "expression" in function_arguments(function)
 
-    # If you update this, you must also update traced_apply_rule
+    # If you update this, you must also update traced_apply_function
     # (that's in the same file TraceBuiltins is)
-    def apply_rule(
+    def apply_function(
         self, expression: BaseElement, vars: dict, options: dict, evaluation: Evaluation
     ):
         if options and self.check_options:

mathics/main.py

@@ -19,7 +19,7 @@
 import sys
 
 from mathics import __version__, license_string, settings, version_string
-from mathics.builtin.trace import TraceBuiltins, traced_apply_rule
+from mathics.builtin.trace import TraceBuiltins, traced_apply_function
 from mathics.core.atoms import String
 from mathics.core.definitions import Definitions, Symbol, autoload_files
 from mathics.core.evaluation import Evaluation, Output
@@ -385,7 +385,7 @@ def main() -> int:
         extension_modules = default_pymathics_modules
 
     if args.trace_builtins:
-        BuiltinRule.apply_rule = traced_apply_rule
+        BuiltinRule.apply_rule = traced_apply_function
 
         def dump_tracing_stats():
             TraceBuiltins.dump_tracing_stats(sort_by="count", evaluation=None)