From b8355fdbbd26b036f40c76791de6ccceb874b19d Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 4 May 2023 21:44:50 -0400 Subject: [PATCH 01/12] Correct docstring of test_docstrings() function --- tests/unit/test_example_docstrings.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 90d709a2f..09a73de2f 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -9,8 +9,7 @@ def test_docstrings(): """ - Check each example for docstring. - Check each example for valid run instructions. + Check each example for a docstring with correct run instructions. """ ignore_patterns = ["__", "perf_test", "text_loc"] From 657044c7cbf5676c58fba06f9fc746e68923862f Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 4 May 2023 21:48:25 -0400 Subject: [PATCH 02/12] Use Path.read_text to read & close file immediately --- tests/unit/test_example_docstrings.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 09a73de2f..ae0e4afe6 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -39,10 +39,9 @@ def check_code_docstring(path: Path, name: str): path: Path to the file name: Name of module """ - with open(path) as f: - code = ast.parse(f.read()) - docstring = ast.get_docstring(code) - run_line = f"python -m {name}" - # print(f"Checking if example {name} has a run instruction..") - assert docstring is not None, f"{run_line} not in {name} docstring." - assert run_line in docstring, f"{run_line} not in {name} docstring." + code = ast.parse(path.read_text()) + docstring = ast.get_docstring(code) + run_line = f"python -m {name}" + # print(f"Checking if example {name} has a run instruction..") + assert docstring is not None, f"{run_line} not in {name} docstring." + assert run_line in docstring, f"{run_line} not in {name} docstring." From aef6b50389da53ee80bcef6cdef05cd4ae38e557 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Thu, 4 May 2023 21:51:22 -0400 Subject: [PATCH 03/12] Add detailed docstring to check_code_docstring --- tests/unit/test_example_docstrings.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index ae0e4afe6..d48407fc8 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -36,12 +36,33 @@ def check_submodules(module_path: str): def check_code_docstring(path: Path, name: str): """ - path: Path to the file - name: Name of module + Read & check a single file for an appropriate docstring + + A docstring should consist of the following: + + 1. A summary line explaining what it demonstrates per PEP-0257 + (https://peps.python.org/pep-0257/#multi-line-docstrings) + 2. If necessary, a further minimal explanation of how it will do so + 3. A line specifying how this example can be as a module run, usually at + the end + + Example:: + + \"\"\" + Show a timer on screen + + If Python and Arcade are installed, this example can be run from the command line with: + python -m arcade.examples.sprite_rooms + \"\"\" + + :param path: Path to the file + :param name: Name of module """ code = ast.parse(path.read_text()) docstring = ast.get_docstring(code) run_line = f"python -m {name}" + # print(f"Checking if example {name} has a run instruction..") assert docstring is not None, f"{run_line} not in {name} docstring." assert run_line in docstring, f"{run_line} not in {name} docstring." + From bd688723c20b0fa0b441d04fd23b35e0fa4b1d54 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 00:58:22 -0400 Subject: [PATCH 04/12] Rename check function to be clearer --- tests/unit/test_example_docstrings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index d48407fc8..68c764950 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -31,10 +31,10 @@ def check_submodules(module_path: str): module = importlib.import_module(module_path) for finder, name, is_pkg in pkgutil.iter_modules(module.__path__): path = Path(finder.path) / f"{name}.py" - check_code_docstring(path, f"{module_path}.{name}") + check_single_example_docstring(path, f"{module_path}.{name}") -def check_code_docstring(path: Path, name: str): +def check_single_example_docstring(path: Path, name: str): """ Read & check a single file for an appropriate docstring From cecb99c8918144de7b40e031c3afd6cb343666eb Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 00:59:58 -0400 Subject: [PATCH 05/12] Rearrange function body & add comment explaining read_text function --- tests/unit/test_example_docstrings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 68c764950..cda9464da 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -58,11 +58,13 @@ def check_single_example_docstring(path: Path, name: str): :param path: Path to the file :param name: Name of module """ + + # Read the file & extract the docstring code = ast.parse(path.read_text()) docstring = ast.get_docstring(code) - run_line = f"python -m {name}" # print(f"Checking if example {name} has a run instruction..") + run_line = f"python -m {name}" assert docstring is not None, f"{run_line} not in {name} docstring." assert run_line in docstring, f"{run_line} not in {name} docstring." From 9f57f9aaf41638c79c17f7635b56abcfdc9f9c2e Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 01:00:31 -0400 Subject: [PATCH 06/12] Clean up whitespace at beginning & end of file --- tests/unit/test_example_docstrings.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index cda9464da..834df0eb1 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -67,4 +67,3 @@ def check_single_example_docstring(path: Path, name: str): run_line = f"python -m {name}" assert docstring is not None, f"{run_line} not in {name} docstring." assert run_line in docstring, f"{run_line} not in {name} docstring." - From c674bb5ce0403038c01b812af378c21dffc5a741 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 01:04:01 -0400 Subject: [PATCH 07/12] Move skipped patterns constant to a top-level tuple --- tests/unit/test_example_docstrings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 834df0eb1..729c9d713 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -4,14 +4,15 @@ import pkgutil import arcade.examples + EXAMPLE_ROOT = "arcade.examples" +SKIP_FILENAME_PATTERNS = ("__", "perf_test", "text_loc") def test_docstrings(): """ Check each example for a docstring with correct run instructions. """ - ignore_patterns = ["__", "perf_test", "text_loc"] # Get all the modules in the examples directory check_submodules(EXAMPLE_ROOT) @@ -21,7 +22,7 @@ def test_docstrings(): if not path.is_dir(): continue - if any(pattern in path.name for pattern in ignore_patterns): + if any(pattern in path.name for pattern in SKIP_FILENAME_PATTERNS): continue check_submodules(f"{EXAMPLE_ROOT}.{path.name}") From 833bd3c000fcddb76d53f40e05bb66b25be89b73 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 01:05:23 -0400 Subject: [PATCH 08/12] Rearrange test_example_docstrings in dependency order --- tests/unit/test_example_docstrings.py | 52 +++++++++++++-------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 729c9d713..9a11eab0d 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -9,32 +9,6 @@ SKIP_FILENAME_PATTERNS = ("__", "perf_test", "text_loc") -def test_docstrings(): - """ - Check each example for a docstring with correct run instructions. - """ - - # Get all the modules in the examples directory - check_submodules(EXAMPLE_ROOT) - - # Check subdirectories - for path in Path(arcade.examples.__path__[0]).iterdir(): - if not path.is_dir(): - continue - - if any(pattern in path.name for pattern in SKIP_FILENAME_PATTERNS): - continue - - check_submodules(f"{EXAMPLE_ROOT}.{path.name}") - - -def check_submodules(module_path: str): - module = importlib.import_module(module_path) - for finder, name, is_pkg in pkgutil.iter_modules(module.__path__): - path = Path(finder.path) / f"{name}.py" - check_single_example_docstring(path, f"{module_path}.{name}") - - def check_single_example_docstring(path: Path, name: str): """ Read & check a single file for an appropriate docstring @@ -68,3 +42,29 @@ def check_single_example_docstring(path: Path, name: str): run_line = f"python -m {name}" assert docstring is not None, f"{run_line} not in {name} docstring." assert run_line in docstring, f"{run_line} not in {name} docstring." + + +def check_submodules(module_path: str): + module = importlib.import_module(module_path) + for finder, name, is_pkg in pkgutil.iter_modules(module.__path__): + path = Path(finder.path) / f"{name}.py" + check_single_example_docstring(path, f"{module_path}.{name}") + + +def test_docstrings(): + """ + Check each example for a docstring with correct run instructions. + """ + + # Get all the modules in the examples directory + check_submodules(EXAMPLE_ROOT) + + # Check subdirectories + for path in Path(arcade.examples.__path__[0]).iterdir(): + if not path.is_dir(): + continue + + if any(pattern in path.name for pattern in SKIP_FILENAME_PATTERNS): + continue + + check_submodules(f"{EXAMPLE_ROOT}.{path.name}") From 53527142bef725bc301c532bc1eb3f185c779c58 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 01:06:11 -0400 Subject: [PATCH 09/12] Add type annotations to helper functions --- tests/unit/test_example_docstrings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 9a11eab0d..cb5ba69d3 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -9,7 +9,7 @@ SKIP_FILENAME_PATTERNS = ("__", "perf_test", "text_loc") -def check_single_example_docstring(path: Path, name: str): +def check_single_example_docstring(path: Path, name: str) -> None: """ Read & check a single file for an appropriate docstring @@ -44,7 +44,7 @@ def check_single_example_docstring(path: Path, name: str): assert run_line in docstring, f"{run_line} not in {name} docstring." -def check_submodules(module_path: str): +def check_submodules(module_path: str) -> None: module = importlib.import_module(module_path) for finder, name, is_pkg in pkgutil.iter_modules(module.__path__): path = Path(finder.path) / f"{name}.py" From c6c8114223ba10c7b710bd1f46f84a3e5f735004 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 01:34:41 -0400 Subject: [PATCH 10/12] Refactor & document check_submodules to remove ambiguity --- tests/unit/test_example_docstrings.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index cb5ba69d3..f21fc10be 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -44,11 +44,28 @@ def check_single_example_docstring(path: Path, name: str) -> None: assert run_line in docstring, f"{run_line} not in {name} docstring." -def check_submodules(module_path: str) -> None: - module = importlib.import_module(module_path) - for finder, name, is_pkg in pkgutil.iter_modules(module.__path__): - path = Path(finder.path) / f"{name}.py" - check_single_example_docstring(path, f"{module_path}.{name}") +def check_submodules(parent_module_absolute_name: str) -> None: + """ + Check docstrings for all immediate child modules of the passed absolute name + + It is important to understand that module names and file paths are different things: + + * A module name is what Python sees the module's name as (``"arcade.color"``) + * A file path is the location on disk (``C:|Users\Reader\python_project\game.py``) + + :param parent_module_absolute_name: The absolute import name of the module to check. + """ + # Get the file system location of the named parent module + parent_module_info = importlib.import_module(parent_module_absolute_name) + parent_module_file_path = parent_module_info.__path__ + + # Check all modules nested immediately inside it on the file system + for finder, child_module_name, is_pkg in pkgutil.iter_modules(parent_module_file_path): + + child_module_file_path = Path(finder.path) / f"{child_module_name}.py" + child_module_absolute_name = f"{parent_module_absolute_name}.{child_module_name}" + + check_single_example_docstring(child_module_file_path, child_module_absolute_name) def test_docstrings(): From 6a162763c15749298fed998a1635733503c1eea6 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 5 May 2023 01:41:29 -0400 Subject: [PATCH 11/12] Eliminate top-level constant due to single use and higher readability when inlined --- tests/unit/test_example_docstrings.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index f21fc10be..5fbc11b5c 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -6,7 +6,6 @@ EXAMPLE_ROOT = "arcade.examples" -SKIP_FILENAME_PATTERNS = ("__", "perf_test", "text_loc") def check_single_example_docstring(path: Path, name: str) -> None: @@ -81,7 +80,7 @@ def test_docstrings(): if not path.is_dir(): continue - if any(pattern in path.name for pattern in SKIP_FILENAME_PATTERNS): + if any(pattern in path.name for pattern in ("__", "perf_test", "text_loc")): continue check_submodules(f"{EXAMPLE_ROOT}.{path.name}") From fde45d5972119d3ec598142cea74a342f0cb2fb9 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Sat, 6 May 2023 02:55:41 -0400 Subject: [PATCH 12/12] Clarify unit test handling docstring run line checks --- tests/unit/test_example_docstrings.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_example_docstrings.py b/tests/unit/test_example_docstrings.py index 5fbc11b5c..0a542c59d 100644 --- a/tests/unit/test_example_docstrings.py +++ b/tests/unit/test_example_docstrings.py @@ -69,18 +69,25 @@ def check_submodules(parent_module_absolute_name: str) -> None: def test_docstrings(): """ - Check each example for a docstring with correct run instructions. + Ensure each user-facing example has a docstring with correct run instructions. """ - # Get all the modules in the examples directory + # Check all immediate child python files in arcade.examples check_submodules(EXAMPLE_ROOT) - # Check subdirectories - for path in Path(arcade.examples.__path__[0]).iterdir(): - if not path.is_dir(): + # For each immediate child folder module in arcade.examples, + # check the immediate child python files for correct docstrings. + for folder_submodule_path in Path(arcade.examples.__path__[0]).iterdir(): + + # Skip file modules we already covered above outside the loop + if not folder_submodule_path.is_dir(): continue - if any(pattern in path.name for pattern in ("__", "perf_test", "text_loc")): + folder_submodule_name = folder_submodule_path.name + + # Skip anything which isn't a user-facing example + if any(pattern in folder_submodule_name for pattern in ("__", "perf_test", "text_loc")): continue - check_submodules(f"{EXAMPLE_ROOT}.{path.name}") + # Check the grandchildren inside the child folder module + check_submodules(f"{EXAMPLE_ROOT}.{folder_submodule_name}")