From 2d15502b160d1bef2d4a5c53964fa666885710f4 Mon Sep 17 00:00:00 2001 From: rocky Date: Sat, 8 Jul 2023 18:34:24 -0400 Subject: [PATCH 1/3] WIP - move more code out of mathics.builtin.init --- mathics/builtin/__init__.py | 69 ++++++---------------------- mathics/core/load_builtin.py | 89 ++++++++++++++++++++++++++++-------- 2 files changed, 84 insertions(+), 74 deletions(-) diff --git a/mathics/builtin/__init__.py b/mathics/builtin/__init__.py index abf253fe5..92586759a 100755 --- a/mathics/builtin/__init__.py +++ b/mathics/builtin/__init__.py @@ -16,81 +16,40 @@ among other things. """ -import glob -import importlib +import os import os.path as osp -import pkgutil -import re from mathics.builtin.base import Builtin from mathics.core.load_builtin import ( add_builtins, + get_module_filenames, + import_builtin_subdirectories, import_builtins, name_is_builtin_symbol, ) from mathics.settings import ENABLE_FILES_MODULE -# Get a list of files in this directory. We'll exclude from the start -# files with leading characters we don't want like __init__ with its leading underscore. -__py_files__ = [ - osp.basename(f[0:-3]) - for f in glob.glob(osp.join(osp.dirname(__file__), "[a-z]*.py")) -] +builtin_path = osp.dirname(__file__) -# FIXME: redo using importlib since that is probably less fragile. +# Get filenames in this directory of Python modules that contain the +# builtin functions that we need to load and process. exclude_files = {"codetables", "base"} -module_names = [ - f for f in __py_files__ if re.match(r"^[a-z\d]+$", f) if f not in exclude_files -] +module_names = [f for f in get_module_filenames(builtin_path) if f not in exclude_files] modules = [] -import_builtins(modules, module_names) +import_builtins(module_names, modules) -_builtins_list = [] builtins_by_module = {} +# The files_io module handles local file access, reading and writing.. +# In some sandboxed settings, such as running Mathics from as a remote +# server, we disallow local file access. disable_file_module_names = [] if ENABLE_FILES_MODULE else ["files_io"] -for subdir in ( - "arithfns", - "assignments", - "atomic", - "binary", - "box", - "colors", - "directories", - "distance", - "drawing", - "exp_structure", - "fileformats", - "files_io", - "file_operations", - "forms", - "functional", - "image", - "intfns", - "list", - "matrices", - "numbers", - "quantum_mechanics", - "specialfns", - "statistics", - "string", - "testing_expressions", - "vectors", -): - import_name = f"{__name__}.{subdir}" - - if subdir in disable_file_module_names: - continue - - builtin_module = importlib.import_module(import_name) - submodule_names = [ - modname for _, modname, _ in pkgutil.iter_modules(builtin_module.__path__) - ] - # print("XXX3", submodule_names) - import_builtins(modules, submodule_names, subdir) +subdirectories = next(os.walk(builtin_path))[1] +import_builtin_subdirectories(subdirectories, disable_file_module_names, modules) +_builtins_list = [] for module in modules: builtins_by_module[module.__name__] = [] module_vars = dir(module) diff --git a/mathics/core/load_builtin.py b/mathics/core/load_builtin.py index 3766f5bb4..1c07b67b7 100755 --- a/mathics/core/load_builtin.py +++ b/mathics/core/load_builtin.py @@ -9,7 +9,10 @@ import importlib import inspect -from typing import Optional +import os.path as osp +import pkgutil +from glob import glob +from typing import Generator, List, Optional from mathics.core.pattern import pattern_objects from mathics.core.symbols import Symbol @@ -17,6 +20,7 @@ _builtins = {} + # The fact that are importing inside here, suggests add_builtins # should get moved elsewhere. def add_builtins(new_builtins): @@ -74,36 +78,83 @@ def definition_contribute(definitions): definitions.builtin[op] = Definition(name=op) +def get_module_filenames(builtin_path: str) -> Generator: + """Return a generator for a filenames that could be Mathics + (builtin) module names. The Python module names have simple + alphabetic names. We do not want to include + things like __init__ or __pycache__ which + cannot be a file containing Mathics3 module that + needs to be considered. + """ + return (osp.basename(f[0:-3]) for f in glob(osp.join(builtin_path, "[a-z]*.py"))) + + +def import_builtin_module(module_name: str, import_name: str, modules: list): + try: + module = importlib.import_module(import_name) + except Exception as e: + print(e) + print(f" Not able to load {module_name} . Check your installation.") + return None + + if module: + modules.append(module) + + # TODO: When we drop Python 3.7, # module_names can be a List[Literal] -def import_builtins(modules: list, module_names: list, submodule_name=None) -> None: +def import_builtins(module_names: list, modules: list, parent_name=None) -> None: """ Imports the list of Mathics3 Built-in modules so that inside - Mathics3 Builtin Functions, like Plus[], List[] are defined. - """ + Mathics3, the Builtin Functions, like Plus[], List[] are defined. - def import_module(module_name: str, import_name: str): - try: - module = importlib.import_module(import_name) - except Exception as e: - print(e) - print(f" Not able to load {module_name}. Check your installation.") - print(f" mathics.builtin loads from {__file__[:-11]}") - return None + If ``parent_name`` is given we are importing a modules under + some parent module. - if module: - modules.append(module) + Imported modules are added to ``modules`` + """ - if submodule_name: - import_module(submodule_name, f"mathics.builtin.{submodule_name}") + if parent_name: + # We need to import the parent module before any modules + # underneath it. + import_builtin_module(parent_name, f"mathics.builtin.{parent_name}", modules) for module_name in module_names: import_name = ( - f"mathics.builtin.{submodule_name}.{module_name}" - if submodule_name + f"mathics.builtin.{parent_name}.{module_name}" + if parent_name else f"mathics.builtin.{module_name}" ) - import_module(module_name, import_name) + import_builtin_module(module_name, import_name, modules) + + +def import_builtin_subdirectories( + subdirectories: List[str], disable_file_module_names: List[str], modules +): + """ + imports and processes builtins in the Python modules named in ``subdirectories``. + This must be under the ``mathics.builtin`` module. + + ``disable_file_module_names`` is a list of modules to ignore. + ``modules`` stores imported information that we gather. + """ + + for subdir in subdirectories: + # Don't process __pycache__ and things like that. + if subdir.startswith("_"): + continue + + import_name = f"mathics.builtin.{subdir}" + + if subdir in disable_file_module_names: + continue + + builtin_module = importlib.import_module(import_name) + submodule_names = [ + modname for _, modname, _ in pkgutil.iter_modules(builtin_module.__path__) + ] + # print("XXX3", submodule_names) + import_builtins(submodule_names, modules, subdir) def name_is_builtin_symbol(module, name: str) -> Optional[type]: From b6b691bc1929069dfbee2eaec448d4a9be090b13 Mon Sep 17 00:00:00 2001 From: rocky Date: Sun, 9 Jul 2023 10:27:02 -0400 Subject: [PATCH 2/3] More code moved to load_builtin.py Some imports moved as well. is_symbol_name comes from mathics_scanner, not the parser. --- mathics/builtin/__init__.py | 52 +++---------- mathics/builtin/atomic/symbols.py | 4 +- mathics/builtin/scoping.py | 3 +- mathics/core/load_builtin.py | 117 +++++++++++++++++------------- mathics/docpipeline.py | 3 +- mathics/eval/pymathics.py | 3 +- mathics/format/mathml.py | 6 +- 7 files changed, 87 insertions(+), 101 deletions(-) diff --git a/mathics/builtin/__init__.py b/mathics/builtin/__init__.py index 92586759a..946d024ba 100755 --- a/mathics/builtin/__init__.py +++ b/mathics/builtin/__init__.py @@ -19,62 +19,34 @@ import os import os.path as osp -from mathics.builtin.base import Builtin from mathics.core.load_builtin import ( - add_builtins, - get_module_filenames, + add_builtins_from_builtin_modules, + get_module_names, import_builtin_subdirectories, import_builtins, - name_is_builtin_symbol, + initialize_display_operators_set, ) from mathics.settings import ENABLE_FILES_MODULE -builtin_path = osp.dirname(__file__) +# Get import modules in this directory of Python modules that contain +# Mathics3 Builtin class definitions. -# Get filenames in this directory of Python modules that contain the -# builtin functions that we need to load and process. +builtin_path = osp.dirname(__file__) exclude_files = {"codetables", "base"} -module_names = [f for f in get_module_filenames(builtin_path) if f not in exclude_files] - +module_names = get_module_names(builtin_path, exclude_files) modules = [] import_builtins(module_names, modules) -builtins_by_module = {} +# Get import modules in subdirectories of this directory of Python +# modules that contain Mathics3 Builtin class definitions. # The files_io module handles local file access, reading and writing.. # In some sandboxed settings, such as running Mathics from as a remote # server, we disallow local file access. -disable_file_module_names = [] if ENABLE_FILES_MODULE else ["files_io"] +disable_file_module_names = set() if ENABLE_FILES_MODULE else {"files_io"} subdirectories = next(os.walk(builtin_path))[1] import_builtin_subdirectories(subdirectories, disable_file_module_names, modules) -_builtins_list = [] -for module in modules: - builtins_by_module[module.__name__] = [] - module_vars = dir(module) - - for name in module_vars: - builtin_class = name_is_builtin_symbol(module, name) - if builtin_class is not None: - instance = builtin_class(expression=False) - - if isinstance(instance, Builtin): - # This set the default context for symbols in mathics.builtins - if not type(instance).context: - type(instance).context = "System`" - _builtins_list.append((instance.get_name(), instance)) - builtins_by_module[module.__name__].append(instance) - - -new_builtins = _builtins_list - -add_builtins(new_builtins) - -display_operators_set = set() -for modname, builtins in builtins_by_module.items(): - for builtin in builtins: - # name = builtin.get_name() - operator = builtin.get_operator_display() - if operator is not None: - display_operators_set.add(operator) +add_builtins_from_builtin_modules(modules) +initialize_display_operators_set() diff --git a/mathics/builtin/atomic/symbols.py b/mathics/builtin/atomic/symbols.py index 2c94cc3d4..417f22c15 100644 --- a/mathics/builtin/atomic/symbols.py +++ b/mathics/builtin/atomic/symbols.py @@ -8,6 +8,8 @@ import re +from mathics_scanner import is_symbol_name + from mathics.builtin.base import Builtin, PrefixOperator, Test from mathics.core.assignment import get_symbol_values from mathics.core.atoms import String @@ -715,8 +717,6 @@ class Symbol_(Builtin): def eval(self, string, evaluation): "Symbol[string_String]" - from mathics.core.parser import is_symbol_name - text = string.value if is_symbol_name(text): return Symbol(evaluation.definitions.lookup_name(string.value)) diff --git a/mathics/builtin/scoping.py b/mathics/builtin/scoping.py index c5b79cadc..f317f22a6 100644 --- a/mathics/builtin/scoping.py +++ b/mathics/builtin/scoping.py @@ -3,6 +3,7 @@ Scoping Constructs """ +from mathics_scanner import is_symbol_name from mathics.builtin.base import Builtin, Predefined from mathics.core.assignment import get_symbol_list @@ -618,8 +619,6 @@ def eval(self, evaluation: Evaluation): def eval_symbol(self, vars, attributes, evaluation: Evaluation): "Unique[vars_, attributes___]" - from mathics.core.parser import is_symbol_name - attributes = attributes.get_sequence() if len(attributes) > 1: evaluation.message("Unique", "argrx", Integer(len(attributes) + 1)) diff --git a/mathics/core/load_builtin.py b/mathics/core/load_builtin.py index 1c07b67b7..376412412 100755 --- a/mathics/core/load_builtin.py +++ b/mathics/core/load_builtin.py @@ -3,8 +3,7 @@ Code around loading Mathics3 Builtin Functions and Variables. This code loads the top-level definition of a Mathics3 -Builtin, and attributes that have not been segregated elsewhere such -as has been done for one of the other modules listed above. +Builtin. """ import importlib @@ -12,14 +11,15 @@ import os.path as osp import pkgutil from glob import glob -from typing import Generator, List, Optional +from typing import List, Optional from mathics.core.pattern import pattern_objects from mathics.core.symbols import Symbol from mathics.eval.makeboxes import builtins_precedence _builtins = {} - +builtins_by_module = {} +display_operators_set = set() # The fact that are importing inside here, suggests add_builtins # should get moved elsewhere. @@ -50,6 +50,31 @@ def add_builtins(new_builtins): _builtins.update(dict(new_builtins)) +def add_builtins_from_builtin_modules(modules): + # This can be put at the top after mathics.builtin.__init__ + # cleanup is done. + from mathics.builtin.base import Builtin + + builtins_list = [] + for module in modules: + builtins_by_module[module.__name__] = [] + module_vars = dir(module) + + for name in module_vars: + builtin_class = name_is_builtin_symbol(module, name) + if builtin_class is not None: + instance = builtin_class(expression=False) + + if isinstance(instance, Builtin): + # This set the default context for symbols in mathics.builtins + if not type(instance).context: + type(instance).context = "System`" + builtins_list.append((instance.get_name(), instance)) + builtins_by_module[module.__name__].append(instance) + add_builtins(builtins_list) + return builtins_by_module + + def builtins_dict(builtins_by_module): return { builtin.get_name(): builtin @@ -78,77 +103,60 @@ def definition_contribute(definitions): definitions.builtin[op] = Definition(name=op) -def get_module_filenames(builtin_path: str) -> Generator: - """Return a generator for a filenames that could be Mathics - (builtin) module names. The Python module names have simple - alphabetic names. We do not want to include - things like __init__ or __pycache__ which - cannot be a file containing Mathics3 module that - needs to be considered. - """ - return (osp.basename(f[0:-3]) for f in glob(osp.join(builtin_path, "[a-z]*.py"))) - - -def import_builtin_module(module_name: str, import_name: str, modules: list): - try: - module = importlib.import_module(import_name) - except Exception as e: - print(e) - print(f" Not able to load {module_name} . Check your installation.") - return None - - if module: - modules.append(module) +def get_module_names(builtin_path: str, exclude_files: set) -> list: + py_files = [ + osp.basename(f[0:-3]) for f in glob(osp.join(builtin_path, "[a-z]*.py")) + ] + return [f for f in py_files if f not in exclude_files] # TODO: When we drop Python 3.7, # module_names can be a List[Literal] -def import_builtins(module_names: list, modules: list, parent_name=None) -> None: +def import_builtins( + module_names: List[str], modules: list, submodule_name: Optional[str] = None +): """ Imports the list of Mathics3 Built-in modules so that inside - Mathics3, the Builtin Functions, like Plus[], List[] are defined. + Mathics3 Builtin Functions, like Plus[], List[] are defined. + """ - If ``parent_name`` is given we are importing a modules under - some parent module. + def import_module(module_name: str, import_name: str): + try: + module = importlib.import_module(import_name) + except Exception as e: + print(e) + print(f" Not able to load {module_name}. Check your installation.") + print(f" mathics.builtin loads from {__file__[:-11]}") + return None - Imported modules are added to ``modules`` - """ + if module: + modules.append(module) - if parent_name: - # We need to import the parent module before any modules - # underneath it. - import_builtin_module(parent_name, f"mathics.builtin.{parent_name}", modules) + if submodule_name: + import_module(submodule_name, f"mathics.builtin.{submodule_name}") for module_name in module_names: import_name = ( - f"mathics.builtin.{parent_name}.{module_name}" - if parent_name + f"mathics.builtin.{submodule_name}.{module_name}" + if submodule_name else f"mathics.builtin.{module_name}" ) - import_builtin_module(module_name, import_name, modules) + import_module(module_name, import_name) def import_builtin_subdirectories( - subdirectories: List[str], disable_file_module_names: List[str], modules + subdirectories: List[str], disable_file_module_names: set, modules ): """ - imports and processes builtins in the Python modules named in ``subdirectories``. - This must be under the ``mathics.builtin`` module. - - ``disable_file_module_names`` is a list of modules to ignore. - ``modules`` stores imported information that we gather. + Runs import_builtisn on the each subdirectory in ``subdirectories`` that inside + Mathics3 Builtin Functions which are inside mathics.builtins.xxx are defined. """ - for subdir in subdirectories: - # Don't process __pycache__ and things like that. - if subdir.startswith("_"): + if subdir in disable_file_module_names: continue import_name = f"mathics.builtin.{subdir}" - if subdir in disable_file_module_names: - continue - builtin_module = importlib.import_module(import_name) submodule_names = [ modname for _, modname, _ in pkgutil.iter_modules(builtin_module.__path__) @@ -157,6 +165,15 @@ def import_builtin_subdirectories( import_builtins(submodule_names, modules, subdir) +def initialize_display_operators_set(): + for _, builtins in builtins_by_module.items(): + for builtin in builtins: + # name = builtin.get_name() + operator = builtin.get_operator_display() + if operator is not None: + display_operators_set.add(operator) + + def name_is_builtin_symbol(module, name: str) -> Optional[type]: """ Checks if ``name`` should be added to definitions, and return diff --git a/mathics/docpipeline.py b/mathics/docpipeline.py index d2b8aee3b..a3be9bef1 100644 --- a/mathics/docpipeline.py +++ b/mathics/docpipeline.py @@ -22,10 +22,9 @@ import mathics import mathics.settings from mathics import settings, version_string -from mathics.builtin import builtins_by_module from mathics.core.definitions import Definitions from mathics.core.evaluation import Evaluation, Output -from mathics.core.load_builtin import builtins_dict +from mathics.core.load_builtin import builtins_by_module, builtins_dict from mathics.core.parser import MathicsSingleLineFeeder from mathics.doc.common_doc import MathicsMainDocumentation from mathics.eval.pymathics import PyMathicsLoadException, eval_LoadModule diff --git a/mathics/eval/pymathics.py b/mathics/eval/pymathics.py index e4b68ccc4..d5ef66dfb 100644 --- a/mathics/eval/pymathics.py +++ b/mathics/eval/pymathics.py @@ -6,10 +6,9 @@ import inspect import sys -from mathics.builtin import builtins_by_module from mathics.builtin.base import Builtin from mathics.core.definitions import Definitions -from mathics.core.load_builtin import name_is_builtin_symbol +from mathics.core.load_builtin import builtins_by_module, name_is_builtin_symbol # The below set and dictionary are used in document generation # for Pymathics modules. diff --git a/mathics/format/mathml.py b/mathics/format/mathml.py index 2e1fa574f..c3ffbd010 100644 --- a/mathics/format/mathml.py +++ b/mathics/format/mathml.py @@ -8,6 +8,8 @@ import base64 import html +from mathics_scanner import is_symbol_name + from mathics.builtin.box.graphics import GraphicsBox from mathics.builtin.box.graphics3d import Graphics3DBox from mathics.builtin.box.layout import ( @@ -27,7 +29,7 @@ add_conversion_fn, lookup_method as lookup_conversion_method, ) -from mathics.core.parser import is_symbol_name +from mathics.core.load_builtin import display_operators_set as operators from mathics.core.symbols import SymbolTrue @@ -59,8 +61,6 @@ def encode_mathml(text: str) -> str: def string(self, **options) -> str: - from mathics.builtin import display_operators_set as operators - text = self.value number_as_text = options.get("number_as_text", None) From fe103585890fe495228e39127104f32c6b17e0b0 Mon Sep 17 00:00:00 2001 From: rocky Date: Sun, 9 Jul 2023 10:37:48 -0400 Subject: [PATCH 3/3] Adjust for moved imports... And modularity is further improved. --- mathics/doc/common_doc.py | 3 ++- test/consistency-and-style/test_duplicate_builtins.py | 3 ++- test/consistency-and-style/test_summary_text.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/mathics/doc/common_doc.py b/mathics/doc/common_doc.py index 22fb7d48f..38a134f9a 100644 --- a/mathics/doc/common_doc.py +++ b/mathics/doc/common_doc.py @@ -34,6 +34,7 @@ from mathics import builtin, settings from mathics.builtin.base import check_requires_list from mathics.core.evaluation import Message, Print +from mathics.core.load_builtin import builtins_by_module as global_builtins_by_module from mathics.core.util import IS_PYPY from mathics.doc.utils import slugify from mathics.eval.pymathics import pymathics_builtins_by_module, pymathics_modules @@ -621,7 +622,7 @@ def gather_doctest_data(self): ( "Reference of Built-in Symbols", builtin.modules, - builtin.builtins_by_module, + global_builtins_by_module, True, ) ]: diff --git a/test/consistency-and-style/test_duplicate_builtins.py b/test/consistency-and-style/test_duplicate_builtins.py index 9a592a756..2e2a0dec8 100644 --- a/test/consistency-and-style/test_duplicate_builtins.py +++ b/test/consistency-and-style/test_duplicate_builtins.py @@ -8,8 +8,9 @@ import pytest -from mathics.builtin import modules, name_is_builtin_symbol +from mathics.builtin import modules from mathics.builtin.base import Builtin +from mathics.core.load_builtin import name_is_builtin_symbol @pytest.mark.skipif( diff --git a/test/consistency-and-style/test_summary_text.py b/test/consistency-and-style/test_summary_text.py index 2c8eacf23..88b87aa71 100644 --- a/test/consistency-and-style/test_summary_text.py +++ b/test/consistency-and-style/test_summary_text.py @@ -7,8 +7,8 @@ import pytest from mathics import __file__ as mathics_initfile_path -from mathics.builtin import name_is_builtin_symbol from mathics.builtin.base import Builtin +from mathics.core.load_builtin import name_is_builtin_symbol from mathics.doc.common_doc import skip_doc # Get file system path name for mathics.builtin