Merge pull request #714 from Mathics3/Symbol-init-cleanup-revised

Remove "value" from Symbol; add SymbolConstant...

Author: rocky

mathics/builtin/makeboxes.py

@@ -485,7 +485,7 @@ def format_operator(operator) -> Union[String, BaseElement]:
                 return op
             return operator
 
-        precedence = prec.value
+        precedence = prec.value if hasattr(prec, "value") else 0
         grouping = grouping.get_name()
 
         if isinstance(expr, Atom):

mathics/core/symbols.py

@@ -329,57 +329,78 @@ def replace_slots(self, slots, evaluation) -> "Atom":
 
 
 class Symbol(Atom, NumericOperators, EvalMixin):
-    """
-    Note: Symbol is right now used in a couple of ways which in the
-    future may be separated.
+    """A Symbol is a kind of Atom that acts as a symbolic variable.
 
-    A Symbol is a kind of Atom that acts as a symbolic variable or
-    symbolic constant.
+    All Symbols have a name that can be converted to string.
 
-    All Symbols have a name that can be converted to string form.
+    A Variable Symbol is a ``Symbol`` that is associated with a
+    ``Definition`` that has an ``OwnValue`` that determines its
+    evaluation value.
 
-    Inside a session, a Symbol can be associated with a ``Definition``
-    that determines its evaluation value.
+    A Function Symbol, like a Variable Symbol, is a ``Symbol`` that is
+    also associated with a ``Definition``. But it has a ``DownValue``
+    that is used in its evaluation.
 
-    We also have Symbols which are immutable or constant; here the
-    definitions are fixed. The predefined Symbols ``True``, ``False``,
-    and ``Null`` are like this.
+    We also have Symbols which in contrast to Variables Symbols have
+    a constant value that cannot change. System`True and System`False
+    are like this.
 
-    Also there are situations where the Symbol acts like Python's
-    intern() built-in function or Lisp's Symbol without its modifyable
-    property list.  Here, the only attribute we care about is the name
-    which is unique across all mentions and uses, and therefore
-    needs it only to be stored as a single object in the system.
+    These however are in class SymbolConstant. See that class for
+    more information.
 
-    Note that the mathics.core.parser.Symbol works exactly this way.
+    Symbol acts like Python's intern() built-in function or Lisp's
+    Symbol without its modifyable property list.  Here, the only
+    attribute we care about is the value which is unique across all
+    mentions and uses, and therefore needs it only to be stored as a
+    single object in the system.
 
-    This aspect may or may not be true for the Symbolic Variable use case too.
+    Note that the mathics.core.parser.Symbol works exactly this way.
     """
 
     name: str
     hash: str
     sympy_dummy: Any
-    defined_symbols = {}
+
+    # Dictionary of Symbols defined so far.
+    # We use this for object uniqueness.
+    # The key is the Symbol object's string name, and the
+    # diectionary's value is the Mathics object for the Symbol.
+    _symbols = {}
+
     class_head_name = "System`Symbol"
 
     # __new__ instead of __init__ is used here because we want
     # to return the same object for a given "name" value.
-    def __new__(cls, name: str, sympy_dummy=None, value=None):
+    def __new__(cls, name: str, sympy_dummy=None):
         """
-        Allocate an object ensuring that for a given `name` we get back the same object.
+        Allocate an object ensuring that for a given ``name`` and ``cls`` we get back the same object,
+        id(object) is the same and its object.__hash__() is the same.
+
+        SymbolConstant's like System`True and System`False set
+        ``value`` to something other than ``None``.
+
         """
         name = ensure_context(name)
-        self = cls.defined_symbols.get(name, None)
+
+        # A lot of the below code is similar to
+        # the corresponding for numeric constants like Integer, Real.
+        self = cls._symbols.get(name)
+
         if self is None:
-            self = super(Symbol, cls).__new__(cls)
+            self = super().__new__(cls)
             self.name = name
 
+            # Cache object so we don't allocate again.
+            cls._symbols[name] = self
+
             # Set a value for self.__hash__() once so that every time
-            # it is used this is fast.
-            # This tuple with "Symbol" is used to give a different hash
-            # than the hash that would be returned if just string name were
-            # used.
-            self.hash = hash(("Symbol", name))
+            # it is used this is fast. Note that in contrast to the
+            # cached object key, the hash key needs to be unique across *all*
+            # Python objects, so we include the class in the
+            # event that different objects have the same Python value.
+            # For example, this can happen with String constants.
+
+            self.hash = hash((cls, name))
 
             # TODO: revise how we convert sympy.Dummy
             # symbols.
@@ -392,25 +413,8 @@ def __new__(cls, name: str, sympy_dummy=None, value=None):
             # value attribute.
             self.sympy_dummy = sympy_dummy
 
-            # This is something that still I do not undestand:
-            # here we are adding another attribute to this class,
-            # which is not clear where is it going to be used, but
-            # which can be different to None just three specific instances:
-            #  * ``System`True``  ->   True
-            #  * ``System`False`` -> False
-            #  * ``System`Null`` -> None
-            #
-            # My guess is that this property should be set for
-            # ``PredefinedSymbol`` but not for general symbols.
-            #
-            # Like it is now, it looks so misterious as
-            # self.sympy_dummy, for which I have to dig into the
-            # code to see even what type of value should be expected
-            # for it.
-            self._value = value
             self._short_name = strip_context(name)
 
-            cls.defined_symbols[name] = self
         return self
 
     def __eq__(self, other) -> bool:
@@ -631,24 +635,59 @@ def to_sympy(self, **kwargs):
             return sympy.Symbol(sympy_symbol_prefix + self.name)
         return builtin.to_sympy(self, **kwargs)
 
-    @property
-    def value(self) -> Any:
-        return self._value
-
 
-class PredefinedSymbol(Symbol):
+class SymbolConstant(Symbol):
     """
-    A Predefined Symbol of the Mathics system.
+    A Symbol Constant is Symbol of the Mathics system whose value can't
+    be changed and has a corresponding Python representation.
 
-    A Symbol which is defined because it is used somewhere in the
-    Mathics system as a built-in name, Attribute, Property, Option,
-    or a Symbolic Constant.
+    Therefore, like an ``Integer`` constant such as ``Integer0``, we don't
+    need to go through ``Definitions`` to get its Python-equivalent value.
 
-    In contrast to Symbol where the name might not have been added to
-    a list of known Symbol names or where the name might get deleted,
-    this never occurs here.
+    For example for the ``SymbolConstant`` ``System`True``, has its
+    value set to the Python ``True`` value.
+
+    Note this is not the same thing as a Symbolic Constant like ``Pi``,
+    which doesn't have an (exact) Python equivalent representation.
+    Also, Pi *can* be Unprotected and changed, while True, cannot.
+
+    Also note that ``SymbolConstant`` differs from ``Symbol`` in that
+    Symbol has no value field (even when its value happens to be
+    representable in Python. Symbols need to go through Definitions
+    get a Symbol's current value, based on the current context and the
+    state of prior operations on that Symbol/Definition binding.
+
+    In sum, SymbolConstant is partly like Symbol, and partly like
+    Numeric constants.
     """
 
+    # Dictionary of SymbolConstants defined so far.
+    # We use this for object uniqueness.
+    # The key is the SymbolConstant's value, and the
+    # diectionary's value is the Mathics object representing that Python value.
+    _symbol_constants = {}
+
+    # We use __new__ here to unsure that two Integer's that have the same value
+    # return the same object.
+    def __new__(cls, name, value):
+
+        name = ensure_context(name)
+        self = cls._symbol_constants.get(name)
+        if self is None:
+            self = super().__new__(cls, name)
+            self._value = value
+
+            # Cache object so we don't allocate again.
+            self._symbol_constants[name] = self
+
+            # Set a value for self.__hash__() once so that every time
+            # it is used this is fast. Note that in contrast to the
+            # cached object key, the hash key needs to be unique across all
+            # Python objects, so we include the class in the
+            # event that different objects have the same Python value
+            self.hash = hash((cls, name))
+        return self
+
     @property
     def is_literal(self) -> bool:
         """
@@ -676,6 +715,10 @@ def is_uncertain_final_definitions(self, definitions) -> bool:
         """
         return False
 
+    @property
+    def value(self):
+        return self._value
+
 
 def symbol_set(*symbols: Tuple[Symbol]) -> FrozenSet[Symbol]:
     """
@@ -689,10 +732,10 @@ def symbol_set(*symbols: Tuple[Symbol]) -> FrozenSet[Symbol]:
 
 # Symbols used in this module.
 
-# Note, below we are only setting PredefinedSymbol for Symbols which
+# Note, below we are only setting SymbolConstant for Symbols which
 # are both predefined and have the Locked attribute.
 
-# An experiment using PredefinedSymbol("Pi") in the Python code and
+# An experiment using SymbolConstant("Pi") in the Python code and
 # running:
 #    {Pi, Unprotect[Pi];Pi=4; Pi, Pi=.; Pi }
 # show that this does not change the output in any way.
@@ -702,9 +745,9 @@ def symbol_set(*symbols: Tuple[Symbol]) -> FrozenSet[Symbol]:
 # more of the below and in systemsymbols
 # PredefineSymbol.
 
-SymbolFalse = PredefinedSymbol("System`False", value=False)
-SymbolList = PredefinedSymbol("System`List")
-SymbolTrue = PredefinedSymbol("System`True", value=True)
+SymbolFalse = SymbolConstant("System`False", value=False)
+SymbolList = SymbolConstant("System`List", value=list)
+SymbolTrue = SymbolConstant("System`True", value=True)
 
 SymbolAbs = Symbol("Abs")
 SymbolDivide = Symbol("Divide")

mathics/core/systemsymbols.py

@@ -173,6 +173,7 @@
 SymbolRepeated = Symbol("System`Repeated")
 SymbolRepeatedNull = Symbol("System`RepeatedNull")
 SymbolReturn = Symbol("System`Return")
+SymbolRight = Symbol("System`Right")
 SymbolRound = Symbol("System`Round")
 SymbolRow = Symbol("System`Row")
 SymbolRowBox = Symbol("System`RowBox")

mathics/eval/makeboxes.py

@@ -7,7 +7,7 @@
 
 
 import typing
-from typing import Any
+from typing import Any, Dict, Type
 
 from mathics.core.atoms import Complex, Integer, Rational, String, SymbolI
 from mathics.core.convert.expression import to_expression_with_specialization
@@ -41,6 +41,8 @@
     SymbolStandardForm,
 )
 
+builtins_precedence: Dict[Symbol, int] = {}
+
 element_formatters = {}
 
 
@@ -51,18 +53,36 @@ def _boxed_string(string: str, **options):
     return StyleBox(String(string), **options)
 
 
-def eval_makeboxes(self, expr, evaluation, f=SymbolStandardForm):
+def eval_fullform_makeboxes(
+    self, expr, evaluation: Evaluation, form=SymbolStandardForm
+) -> Expression:
     """
-    This function takes the definitions prodived by the evaluation
+    This function takes the definitions provided by the evaluation
     object, and produces a boxed form for expr.
+
+    Basically: MakeBoxes[expr // FullForm]
+    """
+    # This is going to be reimplemented.
+    expr = Expression(SymbolFullForm, expr)
+    return Expression(SymbolMakeBoxes, expr, form).evaluate(evaluation)
+
+
+def eval_makeboxes(
+    self, expr, evaluation: Evaluation, form=SymbolStandardForm
+) -> Expression:
+    """
+    This function takes the definitions provided by the evaluation
+    object, and produces a boxed fullform for expr.
+
+    Basically: MakeBoxes[expr // form]
     """
     # This is going to be reimplemented.
-    return Expression(SymbolMakeBoxes, expr, f).evaluate(evaluation)
+    return Expression(SymbolMakeBoxes, expr, form).evaluate(evaluation)
 
 
 def format_element(
     element: BaseElement, evaluation: Evaluation, form: Symbol, **kwargs
-) -> BaseElement:
+) -> Type[BaseElement]:
     """
     Applies formats associated to the expression, and then calls Makeboxes
     """
@@ -82,14 +102,14 @@ def format_element(
 
 def do_format(
     element: BaseElement, evaluation: Evaluation, form: Symbol
-) -> BaseElement:
+) -> Type[BaseElement]:
     do_format_method = element_formatters.get(type(element), do_format_element)
     return do_format_method(element, evaluation, form)
 
 
 def do_format_element(
     element: BaseElement, evaluation: Evaluation, form: Symbol
-) -> BaseElement:
+) -> Type[BaseElement]:
     """
     Applies formats associated to the expression and removes
     superfluous enclosing formats.
@@ -207,7 +227,7 @@ def format_expr(expr):
 
 def do_format_rational(
     element: BaseElement, evaluation: Evaluation, form: Symbol
-) -> BaseElement:
+) -> Type[BaseElement]:
     if form is SymbolFullForm:
         return do_format_expression(
             Expression(
@@ -232,7 +252,7 @@ def do_format_rational(
 
 def do_format_complex(
     element: BaseElement, evaluation: Evaluation, form: Symbol
-) -> BaseElement:
+) -> Type[BaseElement]:
     if form is SymbolFullForm:
         return do_format_expression(
             Expression(
@@ -260,7 +280,7 @@ def do_format_complex(
 
 def do_format_expression(
     element: BaseElement, evaluation: Evaluation, form: Symbol
-) -> BaseElement:
+) -> Type[BaseElement]:
     # # not sure how much useful is this format_cache
     # if element._format_cache is None:
     #    element._format_cache = {}