feat: cleaning code (#440)

Author: clatapie

.pre-commit-config.yaml

@@ -28,7 +28,7 @@ repos:
       - --max-line-length=100
 
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.4.0
+  rev: v5.0.0
   hooks:
   - id: check-merge-conflict
   - id: debug-statements

src/pyconverter/xml2py/ast_tree.py

@@ -2118,16 +2118,26 @@ def args(self):
         for item in self.raw_args:
             arg = to_py_arg_name(str(item))
 
-            # simply check if we can use this as a valid Python kwarg
-            try:
-                exec(f"{arg} = 1.0")
-            except SyntaxError:
+            if "blank" in arg or arg == "":
+                # if the argument is not a valid Python identifier, then skip it
                 arg = ""
+                args.append(arg)
 
-            if "blank" in arg:
-                arg = ""
+            elif " or " in arg:
+                arg = arg.split(" or ")[0].strip()
+                args.append(arg)
+
+            elif arg in ["...", ". . ."]:
+                # Elipsis, needs to be skipped
+                pass
 
-            args.append(arg)
+            elif arg.isidentifier() is False:
+                raise ValueError(
+                    f"Invalid argument '{arg}' in refname element: {self._element.tag}"
+                )
+
+            else:
+                args.append(arg)
 
         # rename duplicate arguments
         if len(args) != len(set(args)):
@@ -3115,13 +3125,13 @@ def arg_desc(self) -> List[Argument]:
             if len(arguments.py_arg_names) != len(arguments.initial_args):
                 # This function needs a special treatment
                 if arg_file.exists():
-                    with open(arg_file, "r") as f:
+                    with open(arg_file, "r", encoding="utf-8") as f:
                         for line in f:
                             pass
                         last_line = line
                 else:
                     last_line = ""
-                with open(arg_file, "a") as f:
+                with open(arg_file, "a", encoding="utf-8") as f:
                     if last_line != f"py_arg_name : {arguments.py_arg_names}\n":
                         f.write("--------------------------------------------------\n")
                         f.write(f"{self.py_name}: {self.group}\n")

src/pyconverter/xml2py/cli.py

@@ -160,6 +160,8 @@ def version():
     "-r",
     "--run-pre-commit",
     type=click.BOOL,
+    default=False,
+    is_flag=True,
     help="Whether to run the pre-commit hook on the autogenerated package source code.",
 )
 @click.option(

src/pyconverter/xml2py/custom_functions.py

@@ -52,7 +52,7 @@ def get_docstring_lists(filename: str) -> Tuple[list[str], list[str], list[str],
         List containing the library import section.
     """
     lines = None
-    with open(filename, "r") as pyfile:
+    with open(filename, "r", encoding="utf-8") as pyfile:
         lines = pyfile.readlines()
     bool_def = False
     end_def = False

src/pyconverter/xml2py/download.py

@@ -35,10 +35,10 @@ def download(c: ContentFile, out: Path) -> None:
     https://github.com/Nordgaren/Github-Folder-Downloader
 
     """
-    r = requests.get(c.download_url)
+    r = requests.get(c.download_url, timeout=10)
     output_path = out / c.path
     output_path.parent.mkdir(parents=True, exist_ok=True)
-    with open(output_path, "wb") as f:
+    with open(output_path, "wb", encoding="utf-8") as f:
         print(f"downloading {c.path} to {out}")
         f.write(r.content)
 

src/pyconverter/xml2py/formatter.py

@@ -23,19 +23,32 @@
 """This module contains the functions to format the generated docstrings with
 `Black <black_>`_."""
 
-import os
+from pathlib import Path
+
+# Subprocess is needed to run pre-commit hooks.
+import subprocess
 
 
 def run_pre_commit(package_path) -> None:
     """Run `pre-commit <pre-commit_>`_ on the autogenerated package."""
     output = 1
     cur_run = 0
     max_run = 10
+    pre_commit_file = package_path / Path(".pre-commit-config.yaml")
+    if not pre_commit_file.exists():
+        raise FileNotFoundError(f"Pre-commit configuration file not found at {pre_commit_file}.")
     while cur_run < max_run and output != 0:
         cur_run += 1
-        output = os.system(
-            f"pre-commit run --all-files --config {package_path}/.pre-commit-config.yaml"
-        )
+        output = subprocess.run(
+            [
+                "pre-commit",
+                "run",
+                "--all-files",
+                "--config",
+                str(pre_commit_file),
+            ],
+            capture_output=True,
+        ).returncode
     if output != 0:
         raise RuntimeError("Pre-commit failed.")
     else:

src/pyconverter/xml2py/load_xml_doc.py

@@ -33,7 +33,7 @@
 
 
 def link_replacer(file, terms, docu_global, links, base_url, fcache):
-    with open(file, "r") as fid:
+    with open(file, "r", encoding="utf-8") as fid:
         text = fid.read()
         matches = re.findall(r"ENTITY([\S\s]*?)(?=\n)", text)
         for match in matches:
@@ -158,7 +158,7 @@ def load_docu_global(term_path: Path) -> dict:
     docu_ent = term_path / "glb" / "docu_global.ent"
 
     docu_global = {}
-    with open(docu_ent, "r") as fid:
+    with open(docu_ent, "r", encoding="utf-8") as fid:
         lines = fid.read().splitlines()
 
         # have to write our own interperter here since this is non-standard lxml
@@ -230,7 +230,7 @@ def load_terms(
 
     variable_path = term_path / "glb" / variable_file
     if variable_path.is_file():
-        with open(variable_path, "r") as fid:
+        with open(variable_path, "r", encoding="utf-8") as fid:
             lines = fid.read().splitlines()
 
         # have to write our own interperter here since this is non-standard lxml
@@ -251,7 +251,7 @@ def load_terms(
 
     global_terms_path = term_path / "glb" / global_terms_file
     if global_terms_path.is_file():
-        with open(global_terms_path, "r") as fid:
+        with open(global_terms_path, "r", encoding="utf-8") as fid:
             lines = fid.read().splitlines()
 
             for line in lines:
@@ -302,7 +302,7 @@ def load_terms(
     if ent_dir.is_dir():
         isoams_dat = list(ent_dir.glob("*.ent"))
         for filename in isoams_dat:
-            with open(filename, "r") as fid:
+            with open(filename, "r", encoding="utf-8") as fid:
                 lines = fid.read().splitlines()
                 # have to write our own interperter here since this is non-standard lxml
                 for line in lines:
@@ -319,7 +319,7 @@ def load_terms(
     # load group code
     group_code_terms_path = term_path / group_code_file
     if group_code_terms_path.is_file():
-        with open(group_code_terms_path, "r") as fid:
+        with open(group_code_terms_path, "r", encoding="utf-8") as fid:
             lines = fid.read().splitlines()
 
         for line in lines:

src/pyconverter/xml2py/utils/utils.py

@@ -46,7 +46,7 @@ def parse_yaml(yaml_path: Path) -> dict:
         Dictionary with the content of the YAML file.
     """
     if yaml_path.is_file():
-        with open(yaml_path, "r") as file:
+        with open(yaml_path, "r", encoding="utf-8") as file:
             data = yaml.safe_load(file)
         return data
     else:
@@ -228,7 +228,7 @@ def import_handler(
         String before the function definition.
     """
 
-    content = open(filename, "r").read()
+    content = open(filename, "r", encoding="utf-8").read()
     list_imports = list(filter(None, str_before_def.split("\n")))
     for import_line in list_imports:
         if import_line in content:
@@ -237,11 +237,11 @@ def import_handler(
 
     if len(list_imports) > 0:
         str_before_def = "\n".join(list_imports) + "\n\n"
-        with open(filename, "r+") as f:
+        with open(filename, "r+", encoding="utf-8") as f:
             f.seek(0, 0)
             f.write(str_before_def + content + additional_content)
     else:
-        with open(filename, "a") as f:
+        with open(filename, "a", encoding="utf-8") as f:
             f.write(additional_content)
 
 
@@ -300,3 +300,25 @@ def get_refentry(filename: Path) -> list:
     """
     root = fromstring(open(filename, "rb").read())
     return list(root.iterfind(".//refentry"))
+
+
+def is_valid_method(method: str) -> bool:
+    """Check if a method is valid Python code.
+
+    Parameters
+    ----------
+    method: str
+        The method source code as a string.
+        It needs to be already indented for class context.
+
+    Returns
+    -------
+    bool
+        ``True`` if the method is valid Python code, ``False`` otherwise.
+    """
+    dummy_class = f"class Dummy:\n{method}"
+    try:
+        compile(dummy_class, "<string>", "exec")
+        return True
+    except SyntaxError as e:
+        return False

src/pyconverter/xml2py/writer.py

@@ -22,6 +22,7 @@
 
 import logging
 from pathlib import Path
+import py_compile
 import shutil
 from typing import Tuple, Union
 
@@ -38,6 +39,7 @@
     get_library_path,
     get_refentry,
     import_handler,
+    is_valid_method,
 )
 import regex as re
 from tqdm import tqdm
@@ -236,7 +238,7 @@ def write_global__init__file(library_path: Path, config_path: Path) -> None:
 
     init_path = init_folder / "__init__.py"
 
-    with open(init_path, "w") as fid:
+    with open(init_path, "w", encoding="utf-8") as fid:
         fid.write(f"from .{initial_imports} import (\n")
         for dir in library_path.iterdir():
             if dir.is_dir():
@@ -265,7 +267,7 @@ def write__init__file(library_path: Path) -> None:
         if dir.is_dir():
             listdir = list(dir.iterdir())
             if len(listdir) > 0:
-                with open(dir / "__init__.py", "w") as fid:
+                with open(dir / "__init__.py", "w", encoding="utf-8") as fid:
                     fid.write(f"from . import (\n")
                     for file in listdir:
                         if file.name.endswith(".py"):
@@ -415,15 +417,15 @@ def write_source(
             python_name = name_map[initial_command_name]
             path = library_path / f"{python_name}.py"
             python_method = command_obj.to_python(custom_functions, comment_command_dict, indent="")
-            try:
-                exec(python_method)
+            # Check the Python method is valid before writing it to the file
+            if is_valid_method(python_method):
                 with open(path, "w", encoding="utf-8") as fid:
                     fid.write(f"{python_method}\n")
-            except Exception as e:
-                raise RuntimeError(f"Failed to execute {python_name}.py") from e
-
+            else:
+                logging.warning(
+                    f"Invalid Python method for {initial_command_name}: {python_method}"
+                )
     else:
-        import subprocess
 
         package_structure = {}
         all_commands = []
@@ -490,7 +492,7 @@ def write_source(
             for class_name, _ in package_structure[module_name].items():
                 file_path = library_path / module_name / f"{class_name}.py"
                 try:
-                    subprocess.run(["python", str(file_path)])
+                    py_compile.compile(str(file_path))
                 except Exception as e:
                     raise RuntimeError(
                         f"Failed to execute '{python_method}' from '{file_path}'."
@@ -553,7 +555,7 @@ def write_docs(
 
     # Write the main doc file
     doc_src = doc_package_path / "docs.rst"
-    with open(doc_src, "w") as fid:
+    with open(doc_src, "w", encoding="utf-8") as fid:
         fid.write(doc_src_content)
 
     if package_structure is not None:
@@ -588,7 +590,7 @@ def write_docs(
             module_folder = doc_package_path / module_folder_name
             module_folder.mkdir(parents=True, exist_ok=True)
             module_file = module_folder / "index.rst"
-            with open(module_file, "w") as fid:
+            with open(module_file, "w", encoding="utf-8") as fid:
                 fid.write(module_content)
 
             for class_file_name, (class_name, method_list) in class_map.items():
@@ -616,7 +618,7 @@ def write_docs(
 
                 # Write the class file
                 class_file = module_folder / f"{class_file_name}.rst"
-                with open(class_file, "w") as fid:
+                with open(class_file, "w", encoding="utf-8") as fid:
                     fid.write(class_content)
 
     return doc_src

tests/conftest.py

@@ -50,7 +50,7 @@ def directory_path(ghdir):
     if os.environ.get("ON_CI", "").lower() == "true":
         directory_path = Path(ghdir) / "mapdl-cmd-doc"
     else:
-        directory_path = Path.cwd().parent / "mapdl-cmd-doc"
+        directory_path = Path.cwd().parent.parent / "ansys-internal" / "mapdl-cmd-doc"
         directory_path = directory_path.resolve()
     return directory_path