Merge branch 'Symbol-init-cleanup' into HEAD

Author: mmatera

mathics/builtin/makeboxes.py

@@ -451,12 +451,17 @@ def eval_postprefix(self, p, expr, h, prec, f, evaluation):
         else:
             return MakeBoxes(expr, f).evaluate(evaluation)
 
+    # FIXME: prec is sometimes not an Integer for a ListExpession.
+    # And this is recent with respect to PredefinedSymbol revision.
+    # How does this get set and why?
     def eval_infix(
         self, expr, operator, prec: Integer, grouping, form: Symbol, evaluation
     ):
         """MakeBoxes[Infix[expr_, operator_, prec_:None, grouping_:None],
         form:StandardForm|TraditionalForm|OutputForm|InputForm]"""
 
+        assert isinstance(prec, Integer)
+
         ## FIXME: this should go into a some formatter.
         def format_operator(operator) -> Union[String, BaseElement]:
             """

mathics/core/symbols.py

@@ -332,57 +332,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 PredefinedSymbol. 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.
+
+        PredefinedSymbol'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.
@@ -395,25 +416,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:
@@ -634,24 +638,47 @@ 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):
     """
-    A Predefined Symbol of the Mathics system.
+    A Predefined Symbol is a Constant Symbol of the Mathics system whose value can't
+    be changed.
 
-    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 say, an Integer constant, we don't need to go
+    through Definitions to get its 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.
+    As we do in numeric constants, a PredefinedSymbols Python-equivalent value not its string name
+    but to its Python-equivalent value. For example for the predefined System`True we
+    we can set its value to the Python ``True`` value.
     """
 
+    # Dictionary of PredefinedSymbols defined so far.
+    # We use this for object uniqueness.
+    # The key is the PredefinedSymbol object's name, and the
+    # diectionary's value is the Mathics object representing that Python value.
+    _predefined_symbols = {}
+
+    # 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._predefined_symbols.get(name)
+        if self is None:
+            self = super().__new__(cls, name)
+            self._value = value
+
+            # Cache object so we don't allocate again.
+            self._predefined_symbols[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:
         """
@@ -679,6 +706,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]:
     """
@@ -706,7 +737,7 @@ def symbol_set(*symbols: Tuple[Symbol]) -> FrozenSet[Symbol]:
 # PredefineSymbol.
 
 SymbolFalse = PredefinedSymbol("System`False", value=False)
-SymbolList = PredefinedSymbol("System`List")
+SymbolList = PredefinedSymbol("System`List", value=list)
 SymbolTrue = PredefinedSymbol("System`True", value=True)
 
 SymbolAbs = Symbol("Abs")