Use ".value" in expr.to_python() and more often (#1417)

Make use of the ".value" field in BaseElement objects, especially
Expression.to_python().

Recall, the parser creates Python equivalents for certain literal
values. In particular, a list of strings. For this, a Python tuple of
`str` appears. And this kind of thing happens a lot. And note that in
contrast to Mathics3 strings, a Python string can be duplicated. So by
using `.value` of strings found in parsing, we are not only saving time
in creation by not copying, but also memory. And we save time in
`to_python()` access as well. In some cases, like datetime objects, we
can just skip `to_python()` and use `.value` instead.

This motivation for this came from a desire to do a similar thing for
SymPy and NumPy expressions, but that will have to wait.

In the parser, list element literals are `tuple` types, whereas
`to_python ()` returns `lists`. In the places where a Python list for a
literal was expected, we have a Python tuple for the literal. In these
cases, the code has been adjusted to allow both tuple and list types.
Where certain list-only operations like mutation were needed, tuples are
converted to lists in a new variable.

Methods in module `mathics.core.atoms` have been put in alphabetical
order. A bug in `convert_expression_elements` in setting `.value` was
fixed. I suspect it wasn't noticed before because we were not making use
of `.value`.

Some of the `FilePrint` error messages have been aligned better for
WMA's (and this makes the errors clearer, too.)

Author: rocky

mathics/builtin/datentime.py

@@ -13,7 +13,7 @@
 import sys
 import time
 from datetime import datetime, timedelta
-from typing import Optional
+from typing import Optional, Union
 
 import dateutil.parser
 
@@ -97,25 +97,18 @@
 
 EPOCH_START = datetime(1900, 1, 1)
 
-if not hasattr(timedelta, "total_seconds"):
-
-    def total_seconds(td):
-        return (
-            float(td.microseconds + (td.seconds + td.days * 24 * 3600) * 10**6)
-            / 10**6
-        )
-
-else:
-    total_seconds = timedelta.total_seconds
+total_seconds = timedelta.total_seconds
 
 SymbolDateObject = Symbol("DateObject")
 SymbolDateString = Symbol("DateString")
 SymbolGregorian = Symbol("Gregorian")
 
 
 class _Date:
-    def __init__(self, datelist=[], absolute=None, datestr=None):
-        datelist += [1900, 1, 1, 0, 0, 0.0][len(datelist) :]
+    def __init__(
+        self, datelist_arg: Union[list, tuple] = [], absolute=None, datestr=None
+    ):
+        datelist = list(datelist_arg) + [1900, 1, 1, 0, 0, 0.0][len(datelist_arg) :]
         self.date = datetime(
             datelist[0],
             datelist[1],
@@ -249,7 +242,7 @@ def to_datelist(self, epochtime, evaluation: Evaluation):
             ]
             return datelist
 
-        if not isinstance(etime, list):
+        if not isinstance(etime, (list, tuple)):
             evaluation.message(form_name, "arg", etime)
             return
 
@@ -258,7 +251,7 @@ def to_datelist(self, epochtime, evaluation: Evaluation):
             for i, val in enumerate(etime)
         ):
             default_date = [1900, 1, 1, 0, 0, 0.0]
-            datelist = etime + default_date[len(etime) :]
+            datelist = list(etime) + default_date[len(etime) :]
             prec_part, imprec_part = datelist[:2], datelist[2:]
 
             try:
@@ -289,12 +282,16 @@ def to_datelist(self, epochtime, evaluation: Evaluation):
         if len(etime) == 2:
             if (
                 isinstance(etime[0], str)
-                and isinstance(etime[1], list)  # noqa
+                and isinstance(etime[1], (list, tuple))  # noqa
                 and all(isinstance(s, str) for s in etime[1])
             ):
                 is_spec = [
                     str(s).strip('"') in DATE_STRING_FORMATS.keys() for s in etime[1]
                 ]
+
+                if isinstance(etime, tuple):
+                    etime = list(etime)
+
                 etime[1] = [str(s).strip('"') for s in etime[1]]
 
                 if sum(is_spec) == len(is_spec):
@@ -389,7 +386,7 @@ def eval_spec(self, epochtime, evaluation: Evaluation) -> Optional[MachineReal]:
         if datelist is None:
             return
 
-        date = _Date(datelist=datelist)
+        date = _Date(datelist_arg=datelist)
         tdelta = date.date - EPOCH_START
         if tdelta.microseconds == 0:
             return Integer(int(total_seconds(tdelta)))
@@ -487,8 +484,8 @@ def eval(
         # Process dates
         pydate1, pydate2 = date1.to_python(), date2.to_python()
 
-        if isinstance(pydate1, list):  # Date List
-            idate = _Date(datelist=pydate1)
+        if isinstance(pydate1, (list, tuple)):  # Date List
+            idate = _Date(datelist_arg=pydate1)
         elif isinstance(pydate1, (float, int)):  # Absolute Time
             idate = _Date(absolute=pydate1)
         elif isinstance(pydate1, str):  # Date string
@@ -497,8 +494,8 @@ def eval(
             evaluation.message("DateDifference", "date", date1)
             return
 
-        if isinstance(pydate2, list):  # Date List
-            fdate = _Date(datelist=pydate2)
+        if isinstance(pydate2, (list, tuple)):  # Date List
+            fdate = _Date(datelist_arg=pydate2)
         elif isinstance(pydate2, (int, float)):  # Absolute Time
             fdate = _Date(absolute=pydate2)
         elif isinstance(pydate1, str):  # Date string
@@ -517,7 +514,9 @@ def eval(
         pyunits = units.to_python()
         if isinstance(pyunits, str):
             pyunits = [str(pyunits.strip('"'))]
-        elif isinstance(pyunits, list) and all(isinstance(p, str) for p in pyunits):
+        elif isinstance(pyunits, (list, tuple)) and all(
+            isinstance(p, str) for p in pyunits
+        ):
             pyunits = [p.strip('"') for p in pyunits]
 
         if not all(p in TIME_INCREMENTS.keys() for p in pyunits):
@@ -762,9 +761,9 @@ def eval(
 
         # Process date
         pydate = date.to_python()
-        if isinstance(pydate, list):
+        if isinstance(pydate, (list, tuple)):
             date_prec = len(pydate)
-            idate = _Date(datelist=pydate)
+            idate = _Date(datelist_arg=pydate)
         elif isinstance(pydate, float) or isinstance(pydate, int):
             date_prec = "absolute"
             idate = _Date(absolute=pydate)
@@ -779,13 +778,17 @@ def eval(
         pyoff = off.to_python()
         if isinstance(pyoff, float) or isinstance(pyoff, int):
             pyoff = [[pyoff, '"Day"']]
-        elif isinstance(pyoff, list) and len(pyoff) == 2 and isinstance(pyoff[1], str):
+        elif (
+            isinstance(pyoff, (list, tuple))
+            and len(pyoff) == 2
+            and isinstance(pyoff[1], str)
+        ):
             pyoff = [pyoff]
 
         # Strip " marks
         pyoff = [[x[0], x[1].strip('"')] for x in pyoff]
 
-        if isinstance(pyoff, list) and all(  # noqa
+        if isinstance(pyoff, (list, tuple)) and all(  # noqa
             len(o) == 2
             and o[1] in TIME_INCREMENTS.keys()
             and isinstance(o[0], (float, int))
@@ -940,15 +943,16 @@ def eval(
         self, epochtime: BaseElement, form: BaseElement, evaluation: Evaluation
     ) -> Optional[String]:
         "DateString[epochtime_, form_]"
+
         datelist = self.to_datelist(epochtime, evaluation)
 
         if datelist is None:
             return
 
-        date = _Date(datelist=datelist)
+        date = _Date(datelist_arg=datelist)
 
         pyform = form.to_python()
-        if not isinstance(pyform, list):
+        if not isinstance(pyform, (list, tuple)):
             pyform = [pyform]
 
         pyform = [x.strip('"') for x in pyform]

mathics/builtin/directories/directory_names.py

@@ -177,14 +177,19 @@ class FileNameJoin(Builtin):
     def eval(self, pathlist, evaluation: Evaluation, options: dict):
         "FileNameJoin[pathlist_List, OptionsPattern[FileNameJoin]]"
 
+        # Convert pathlist to a Python list, and strip leading and trailing
+        # quotes if that appears.
         py_pathlist = pathlist.to_python()
-        if not all(isinstance(p, str) and p[0] == p[-1] == '"' for p in py_pathlist):
+
+        if not all(isinstance(p, str) for p in py_pathlist):
             return
-        py_pathlist = [p[1:-1] for p in py_pathlist]
 
-        operating_system = (
-            options["System`OperatingSystem"].evaluate(evaluation).get_string_value()
-        )
+        if isinstance(py_pathlist, tuple):
+            py_pathlist = list(py_pathlist)
+        else:
+            py_pathlist = [p[1:-1] if p[0] == p[-1] == '"' else p for p in py_pathlist]
+
+        operating_system = options["System`OperatingSystem"].evaluate(evaluation).value
 
         if operating_system not in ["MacOSX", "Windows", "Unix"]:
             evaluation.message(

mathics/builtin/file_operations/file_properties.py

@@ -4,21 +4,18 @@
 
 import os
 import os.path as osp
-import time
+from datetime import datetime
 
 from mathics.builtin.exp_structure.size_and_sig import Hash
 from mathics.builtin.files_io.files import MathicsOpen
-from mathics.core.atoms import Real, String
+from mathics.core.atoms import String
 from mathics.core.attributes import A_PROTECTED, A_READ_PROTECTED
 from mathics.core.builtin import Builtin, MessageException
 from mathics.core.convert.expression import to_expression
-from mathics.core.convert.python import from_python
 from mathics.core.evaluation import Evaluation
-from mathics.core.expression import Expression
 from mathics.core.streams import path_search
 from mathics.core.symbols import Symbol, SymbolNull
-from mathics.core.systemsymbols import SymbolAbsoluteTime, SymbolFailed, SymbolNone
-from mathics.eval.nevaluator import eval_N
+from mathics.core.systemsymbols import SymbolFailed, SymbolNone
 
 sort_order = "mathics.builtin.file-operations.file_properties"
 
@@ -103,17 +100,17 @@ def eval(self, path, timetype, evaluation):
             evaluation.message("FileDate", "datetype")
             return
 
-        # Offset for system epoch
-        epochtime_expr = Expression(
-            SymbolAbsoluteTime, String(time.strftime("%Y-%m-%d %H:%M", time.gmtime(0)))
+        dt_object = datetime.fromtimestamp(result)
+        # Extract the year, month, day, hour, and minute into a tuple
+        datetime_tuple = (
+            dt_object.year,
+            dt_object.month,
+            dt_object.day,
+            dt_object.hour,
+            dt_object.minute,
+            dt_object.second,
         )
-        epochtime_N = eval_N(epochtime_expr, evaluation)
-        if epochtime_N is None:
-            return None
-        epochtime = epochtime_N.to_python()
-        result += epochtime
-
-        return to_expression("DateList", Real(result))
+        return to_expression("DateList", datetime_tuple)
 
     def eval_default(self, path, evaluation):
         "FileDate[path_]"
@@ -299,7 +296,7 @@ def eval(self, filename, datelist, attribute, evaluation):
 
         # Check datelist
         if not (
-            isinstance(py_datelist, list)
+            isinstance(py_datelist, (list, tuple))
             and len(py_datelist) == 6
             and all(isinstance(d, int) for d in py_datelist[:-1])
             and isinstance(py_datelist[-1], float)
@@ -311,25 +308,13 @@ def eval(self, filename, datelist, attribute, evaluation):
             evaluation.message("SetFileDate", "datetype")
             return
 
-        epochtime = (
-            to_expression(
-                "AbsoluteTime", time.strftime("%Y-%m-%d %H:%M", time.gmtime(0))
-            )
-            .evaluate(evaluation)
-            .to_python()
-        )
-
-        stattime = to_expression("AbsoluteTime", from_python(py_datelist))
-        stattime_N = eval_N(stattime, evaluation)
-        if stattime_N is None:
-            return
-
-        stattime = stattime_N.to_python() - epochtime
+        file_timestamp = datetime(*py_datelist[:4]).timestamp()
 
         try:
             os.stat(py_filename)
             if py_attr == '"Access"':
-                os.utime(py_filename, (stattime, osp.getatime(py_filename)))
+                os.utime(py_filename, (file_timestamp, osp.getatime(py_filename)))
+                return SymbolNull
             if py_attr == '"Creation"':
                 if os.name == "posix":
                     evaluation.message("SetFileDate", "nocreationunix")
@@ -338,9 +323,9 @@ def eval(self, filename, datelist, attribute, evaluation):
                     # TODO: Note: This is windows only
                     return SymbolFailed
             if py_attr == '"Modification"':
-                os.utime(py_filename, (osp.getatime(py_filename), stattime))
-            if py_attr == "All":
-                os.utime(py_filename, (stattime, stattime))
+                os.utime(py_filename, (osp.getatime(py_filename), file_timestamp))
+            elif py_attr == "All":
+                os.utime(py_filename, (file_timestamp, file_timestamp))
         except OSError:
             # evaluation.message(...)
             return SymbolFailed

mathics/builtin/files_io/files.py

@@ -267,9 +267,8 @@ class FilePrint(Builtin):
     """
 
     messages = {
-        "fstr": (
-            "File specification `1` is not a string of " "one or more characters."
-        ),
+        "zstr": ("The file name cannot be an empty string."),
+        "badfile": ("The specified argument, `1`, should be a valid string."),
     }
 
     options = {
@@ -281,33 +280,35 @@ class FilePrint(Builtin):
 
     def eval(self, path, evaluation: Evaluation, options: dict):
         "FilePrint[path_, OptionsPattern[FilePrint]]"
+
+        if not isinstance(path, String):
+            evaluation.message("FilePrint", "badfile", path)
+            return
+
         pypath = path.to_python()
+
         if not (
             isinstance(pypath, str)
             and pypath[0] == pypath[-1] == '"'
             and len(pypath) > 2
         ):
-            evaluation.message("FilePrint", "fstr", path)
+            evaluation.message("FilePrint", "zstr", path)
             return
-        pypath, _ = path_search(pypath[1:-1])
+        resolved_pypath, _ = path_search(pypath[1:-1])
 
         # Options
         record_separators = options["System`RecordSeparators"].to_python()
-        assert isinstance(record_separators, list)
-        assert all(
-            isinstance(s, str) and s[0] == s[-1] == '"' for s in record_separators
-        )
-        record_separators = [s[1:-1] for s in record_separators]
+        assert isinstance(record_separators, tuple)
 
-        if pypath is None:
+        if resolved_pypath is None:
             evaluation.message("General", "noopen", path)
             return
 
-        if not osp.isfile(pypath):
+        if not osp.isfile(resolved_pypath):
             return SymbolFailed
 
         try:
-            with MathicsOpen(pypath, "r") as f:
+            with MathicsOpen(resolved_pypath, "r") as f:
                 result = f.read()
         except IOError:
             evaluation.message("General", "noopen", path)
@@ -1374,18 +1375,26 @@ def eval(self, name, n, text, evaluation: Evaluation, options: dict):
 
         stream = to_expression("InputStream", name, n)
 
-        if not isinstance(py_text, list):
+        if not isinstance(py_text, (list, tuple)):
             py_text = [py_text]
 
-        if not all(isinstance(t, str) and t[0] == t[-1] == '"' for t in py_text):
+        if not all(isinstance(t, str) for t in py_text):
             evaluation.message("Find", "unknown", to_expression("Find", stream, text))
             return
 
-        py_text = [t[1:-1] for t in py_text]
+        # If py_text comes from a (literal) value, then there are no
+        # leading/trailing quotes around strings.  If it is still
+        # possible that py_text can be a list, then there could be
+        # leading/traling quotes.
+        if isinstance(py_text, list):
+            py_text = [t[1:-1] if t[0] == t[-1] == '"' else t for t in py_text]
 
         while True:
             tmp = super(Find, self).eval(stream, Symbol("Record"), evaluation, options)
-            py_tmp = tmp.to_python()[1:-1]
+            if not isinstance(tmp, String):
+                return SymbolFailed
+
+            py_tmp = tmp.value
 
             if py_tmp == "System`EndOfFile":
                 evaluation.message(