From d8f05e6bc6f31bea0229b4b3c0c18ceedb3f3217 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Sun, 6 Apr 2025 00:27:51 +1100 Subject: [PATCH 01/12] fix(validate.py): Considers subclass nesting when checking GL08 constructor Introduced GL08 checking for constructors (i.e. __init__) but didn't traverse the parent class heireacy when importing the __init__ parent from a module. --- numpydoc/validate.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/numpydoc/validate.py b/numpydoc/validate.py index 858a06d2..cfc80adb 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -645,7 +645,9 @@ def validate(obj_name, validator_cls=None, **validator_kwargs): and doc.is_function_or_method and hasattr(doc, "code_obj") ): - cls_name = doc.code_obj.__qualname__.split(".")[0] + cls_name = ".".join( + doc.code_obj.__qualname__.split(".")[:-1] + ) # Collect all class depths before the constructor. cls = Validator._load_obj(f"{doc.code_obj.__module__}.{cls_name}") # cls = Validator._load_obj(f"{doc.name[:-9]}.{cls_name}") ## Alternative cls_doc = Validator(get_doc_object(cls)) From c7da072e22a7012b9d762283ce4c3db261779591 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Sun, 6 Apr 2025 22:08:08 +1000 Subject: [PATCH 02/12] test(validate.py): Added a test to check nested class docstring when checking for initialisation docstring Test written due to bug missed in https://github.com/numpy/numpydoc/pull/592. --- numpydoc/tests/test_validate.py | 80 +++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index 9f0f7942..6cf33d43 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -1305,6 +1305,69 @@ def __init__(self, param1: int): pass +class ConstructorDocumentedinEmbeddedClass: # ignore Gl08, ES01 + """ + Class to test the initialisation behaviour of a embedded class. + """ + + class EmbeddedClass1: # ignore GL08, ES01 + """ + An additional level for embedded class documentation checking. + """ + + class EmbeddedClass2: + """ + This is an embedded class. + + Extended summary. + + Parameters + ---------- + param1 : int + Description of param1. + + See Also + -------- + otherclass : A class that does something else. + + Examples + -------- + This is an example of how to use EmbeddedClass. + """ + + def __init__(self, param1: int) -> None: + pass + + +class IncompleteConstructorDocumentedinEmbeddedClass: + """ + Class to test the initialisation behaviour of a embedded class. + """ + + class EmbeddedClass1: + """ + An additional level for embedded class documentation checking. + """ + + class EmbeddedClass2: + """ + This is an embedded class. + + Extended summary. + + See Also + -------- + otherclass : A class that does something else. + + Examples + -------- + This is an example of how to use EmbeddedClass. + """ + + def __init__(self, param1: int) -> None: + pass + + class TestValidator: def _import_path(self, klass=None, func=None): """ @@ -1660,6 +1723,18 @@ def test_bad_docstrings(self, capsys, klass, func, msgs): tuple(), ("PR01"), # Parameter not documented in class constructor ), + ( + "ConstructorDocumentedinEmbeddedClass.EmbeddedClass1.EmbeddedClass2", + tuple(), + ("GL08",), + tuple(), + ), + ( + "IncompleteConstructorDocumentedinEmbeddedClass.EmbeddedClass1.EmbeddedClass2", + ("GL08",), + tuple(), + ("PR01",), + ), ], ) def test_constructor_docstrings( @@ -1677,6 +1752,11 @@ def test_constructor_docstrings( for code in exc_init_codes: assert code not in " ".join(err[0] for err in result["errors"]) + if klass == "ConstructorDocumentedinEmbeddedClass": + raise NotImplementedError( + "Test for embedded class constructor docstring not implemented yet." + ) + def decorator(x): """Test decorator.""" From 92d83050ec7807b70a7eddbe2a5cf097455c87dd Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Sun, 11 May 2025 16:00:59 +1000 Subject: [PATCH 03/12] fix(validate.py): Allows the validator to check AST constructor docstrings compliance with parent class The abstract syntax tree doesn't provide any link to a parent class at node level, so special traversal is required to check constructor parent implementation of docstrings. --- numpydoc/validate.py | 61 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/numpydoc/validate.py b/numpydoc/validate.py index cfc80adb..9d96c637 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -573,6 +573,14 @@ def _check_desc(desc, code_no_desc, code_no_upper, code_no_period, **kwargs): return errs +def _find_class_node(module_node: ast.AST, cls_name) -> ast.ClassDef: + # Find the class node within a module, when checking constructor docstrings. + for node in ast.walk(module_node): + if isinstance(node, ast.ClassDef) and node.name == cls_name: + return node + raise ValueError(f"Could not find class node {cls_name}") + + def validate(obj_name, validator_cls=None, **validator_kwargs): """ Validate the docstring. @@ -640,22 +648,51 @@ def validate(obj_name, validator_cls=None, **validator_kwargs): report_GL08: bool = True # Check if the object is a class and has a docstring in the constructor # Also check if code_obj is defined, as undefined for the AstValidator in validate_docstrings.py. - if ( - doc.name.endswith(".__init__") - and doc.is_function_or_method - and hasattr(doc, "code_obj") - ): - cls_name = ".".join( - doc.code_obj.__qualname__.split(".")[:-1] - ) # Collect all class depths before the constructor. - cls = Validator._load_obj(f"{doc.code_obj.__module__}.{cls_name}") - # cls = Validator._load_obj(f"{doc.name[:-9]}.{cls_name}") ## Alternative - cls_doc = Validator(get_doc_object(cls)) + if doc.name.endswith(".__init__") and doc.is_function_or_method: + from numpydoc.hooks.validate_docstrings import ( + AstValidator, # Support abstract syntax tree hook. + ) + + if hasattr(doc, "code_obj"): + cls_name = ".".join( + doc.code_obj.__qualname__.split(".")[:-1] + ) # Collect all class depths before the constructor. + cls = Validator._load_obj(f"{doc.code_obj.__module__}.{cls_name}") + # cls = Validator._load_obj(f"{doc.name[:-9]}.{cls_name}") ## Alternative + cls_doc = Validator(get_doc_object(cls)) + elif isinstance(doc, AstValidator): # Supports class traversal for ASTs. + names = doc._name.split(".") + + if len(names) > 2: # i.e. module.class.__init__ + nested_cls_names = names[1:-1] # class1,class2 etc. + cls_name = names[-2] + filename = doc.source_file_name # from the AstValidator object + + # Use AST to find the class node... + with open(filename) as file: + module_node = ast.parse(file.read(), filename) + + # Recursively find each subclass from the module node. + next_node = module_node + for name in nested_cls_names: + next_node = _find_class_node(next_node, name) + # Get the documentation. + cls_doc = AstValidator( + ast_node=next_node, filename=filename, obj_name=cls_name + ) + else: + # Ignore edge case: __init__ functions that don't belong to a class. + cls_doc = None + else: + raise TypeError( + f"Cannot load {doc.name} as a usable Validator object (Validator does not have `doc_obj` attr or type `AstValidator`)." + ) # Parameter_mismatches, PR01, PR02, PR03 are checked for the class docstring. # If cls_doc has PR01, PR02, PR03 errors, i.e. invalid class docstring, # then we also report missing constructor docstring, GL08. - report_GL08 = len(cls_doc.parameter_mismatches) > 0 + if cls_doc: + report_GL08 = len(cls_doc.parameter_mismatches) > 0 # Check if GL08 is to be ignored: if "GL08" in ignore_validation_comments: From 9f38b98235ea973fd9fda9609a108d15c705d64e Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Sun, 11 May 2025 22:56:40 +1000 Subject: [PATCH 04/12] test(test_validate_hook.py,-example_module.py): Wrote new example_module tests for AST (AbstractSyntaxTree) discovery of constructor method parent class. --- numpydoc/tests/hooks/example_module.py | 33 ++++++++++++++++++- numpydoc/tests/hooks/test_validate_hook.py | 38 ++++++++++++++++------ 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/numpydoc/tests/hooks/example_module.py b/numpydoc/tests/hooks/example_module.py index 9f75bdf0..070fea50 100644 --- a/numpydoc/tests/hooks/example_module.py +++ b/numpydoc/tests/hooks/example_module.py @@ -28,4 +28,35 @@ def create(self): class NewClass: - pass + class GoodConstructor: + """ + A nested class to test constructors via AST hook. + + Implements constructor via class docstring. + + Parameters + ---------- + name : str + The name of the new class. + """ + + def __init__(self, name): + self.name = name + + class BadConstructor: + """ + A nested class to test constructors via AST hook. + + Implements a bad constructor docstring despite having a good class docstring. + + Parameters + ---------- + name : str + The name of the new class. + """ + + def __init__(self, name): + """ + A failing constructor implementation without parameters. + """ + self.name = name diff --git a/numpydoc/tests/hooks/test_validate_hook.py b/numpydoc/tests/hooks/test_validate_hook.py index 47f315c2..c97b38d6 100644 --- a/numpydoc/tests/hooks/test_validate_hook.py +++ b/numpydoc/tests/hooks/test_validate_hook.py @@ -1,6 +1,7 @@ """Test the numpydoc validate pre-commit hook.""" import inspect +import os from pathlib import Path import pytest @@ -40,8 +41,6 @@ def test_validate_hook(example_module, config, capsys): numpydoc/tests/hooks/example_module.py:8: EX01 No examples section found - numpydoc/tests/hooks/example_module.py:11: GL08 The object does not have a docstring - numpydoc/tests/hooks/example_module.py:17: ES01 No extended summary found numpydoc/tests/hooks/example_module.py:17: PR01 Parameters {'**kwargs'} not documented @@ -61,8 +60,24 @@ def test_validate_hook(example_module, config, capsys): numpydoc/tests/hooks/example_module.py:26: EX01 No examples section found numpydoc/tests/hooks/example_module.py:30: GL08 The object does not have a docstring + + numpydoc/tests/hooks/example_module.py:31: SA01 See Also section not found + + numpydoc/tests/hooks/example_module.py:31: EX01 No examples section found + + numpydoc/tests/hooks/example_module.py:46: SA01 See Also section not found + + numpydoc/tests/hooks/example_module.py:46: EX01 No examples section found + + numpydoc/tests/hooks/example_module.py:58: ES01 No extended summary found + + numpydoc/tests/hooks/example_module.py:58: PR01 Parameters {'name'} not documented + + numpydoc/tests/hooks/example_module.py:58: SA01 See Also section not found + + numpydoc/tests/hooks/example_module.py:58: EX01 No examples section found """ - ) + ).replace("/", os.sep) return_code = run_hook([example_module], config=config) assert return_code == 1 @@ -79,8 +94,6 @@ def test_validate_hook_with_ignore(example_module, capsys): """ numpydoc/tests/hooks/example_module.py:4: PR01 Parameters {'name'} not documented - numpydoc/tests/hooks/example_module.py:11: GL08 The object does not have a docstring - numpydoc/tests/hooks/example_module.py:17: PR01 Parameters {'**kwargs'} not documented numpydoc/tests/hooks/example_module.py:17: PR07 Parameter "*args" has no description @@ -88,8 +101,10 @@ def test_validate_hook_with_ignore(example_module, capsys): numpydoc/tests/hooks/example_module.py:26: SS05 Summary must start with infinitive verb, not third person (e.g. use "Generate" instead of "Generates") numpydoc/tests/hooks/example_module.py:30: GL08 The object does not have a docstring + + numpydoc/tests/hooks/example_module.py:58: PR01 Parameters {'name'} not documented """ - ) + ).replace("/", os.sep) return_code = run_hook([example_module], ignore=["ES01", "SA01", "EX01"]) @@ -132,7 +147,7 @@ def test_validate_hook_with_toml_config(example_module, tmp_path, capsys): numpydoc/tests/hooks/example_module.py:30: GL08 The object does not have a docstring """ - ) + ).replace("/", os.sep) return_code = run_hook([example_module], config=tmp_path) assert return_code == 1 @@ -167,7 +182,7 @@ def test_validate_hook_with_setup_cfg(example_module, tmp_path, capsys): numpydoc/tests/hooks/example_module.py:30: GL08 The object does not have a docstring """ - ) + ).replace("/", os.sep) return_code = run_hook([example_module], config=tmp_path) assert return_code == 1 @@ -208,7 +223,7 @@ def test_validate_hook_exclude_option_pyproject(example_module, tmp_path, capsys numpydoc/tests/hooks/example_module.py:30: GL08 The object does not have a docstring """ - ) + ).replace("/", os.sep) return_code = run_hook([example_module], config=tmp_path) assert return_code == 1 @@ -241,8 +256,11 @@ def test_validate_hook_exclude_option_setup_cfg(example_module, tmp_path, capsys numpydoc/tests/hooks/example_module.py:17: PR07 Parameter "*args" has no description """ - ) + ).replace("/", os.sep) return_code = run_hook([example_module], config=tmp_path) assert return_code == 1 assert capsys.readouterr().err.strip() == expected + + +# def test_validate_hook_ From af861c3c5baa37b6112029640f51e52e0fb0897a Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Mon, 12 May 2025 00:30:47 +1000 Subject: [PATCH 05/12] ci(test.yml): Added --pre option to prerelease job to ensure pre-release installation, and changed pytest invocation to use `numpydoc` instead of `.`, for consistency with `test` job In response to discussion on #591 --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8db3074c..2a6d7879 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -95,12 +95,12 @@ jobs: - name: Install run: | - python -m pip install .[test,doc] + python -m pip install --pre .[test,doc] pip list - name: Run test suite run: | - pytest -v --pyargs . + pytest -v --pyargs numpydoc - name: Test coverage run: | From c9d238437db5c3e6285ae37de2f48afae12c65c4 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Mon, 12 May 2025 00:59:18 +1000 Subject: [PATCH 06/12] refactor(tests): Remove `__init__.py` module status of `tests\hooks\` to match `tests\` directory --- numpydoc/tests/hooks/__init__.py | 1 - 1 file changed, 1 deletion(-) delete mode 100644 numpydoc/tests/hooks/__init__.py diff --git a/numpydoc/tests/hooks/__init__.py b/numpydoc/tests/hooks/__init__.py deleted file mode 100644 index f2d333a5..00000000 --- a/numpydoc/tests/hooks/__init__.py +++ /dev/null @@ -1 +0,0 @@ -"""Tests for hooks.""" From b62c21fe30f173079cbb6551bb8e43d87c3e7a54 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Mon, 12 May 2025 01:13:53 +1000 Subject: [PATCH 07/12] ci(test.yml): Added explicit call to hook tests to see if included in pytest execution --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2a6d7879..ab0f94dc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -49,6 +49,7 @@ jobs: - name: Run test suite run: | pytest -v --pyargs numpydoc + pytest -v --pyargs numpydoc/tests/hooks - name: Test coverage run: | From becbaeb952096849ebd3785635723ffdaa312f6c Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Tue, 24 Jun 2025 00:38:02 +1000 Subject: [PATCH 08/12] test(tests\hooks\test_validate_hook.py): Changed constructor validation to use AST ancestry, fixed hook tests and added Windows support for test_utils --- numpydoc/tests/hooks/test_utils.py | 4 ++- numpydoc/tests/hooks/test_validate_hook.py | 4 +++ numpydoc/validate.py | 30 ++++++++++------------ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/numpydoc/tests/hooks/test_utils.py b/numpydoc/tests/hooks/test_utils.py index 5db63762..ce2fc021 100644 --- a/numpydoc/tests/hooks/test_utils.py +++ b/numpydoc/tests/hooks/test_utils.py @@ -23,7 +23,9 @@ def test_find_project_root(tmp_path, request, reason_file, files, expected_reaso (tmp_path / reason_file).touch() if files: - expected_dir = Path("/") if expected_reason == "file system root" else tmp_path + expected_dir = ( + Path(tmp_path.anchor) if expected_reason == "file system root" else tmp_path + ) for file in files: (tmp_path / file).touch() else: diff --git a/numpydoc/tests/hooks/test_validate_hook.py b/numpydoc/tests/hooks/test_validate_hook.py index c97b38d6..16c50f8a 100644 --- a/numpydoc/tests/hooks/test_validate_hook.py +++ b/numpydoc/tests/hooks/test_validate_hook.py @@ -41,6 +41,8 @@ def test_validate_hook(example_module, config, capsys): numpydoc/tests/hooks/example_module.py:8: EX01 No examples section found + numpydoc/tests/hooks/example_module.py:11: GL08 The object does not have a docstring + numpydoc/tests/hooks/example_module.py:17: ES01 No extended summary found numpydoc/tests/hooks/example_module.py:17: PR01 Parameters {'**kwargs'} not documented @@ -94,6 +96,8 @@ def test_validate_hook_with_ignore(example_module, capsys): """ numpydoc/tests/hooks/example_module.py:4: PR01 Parameters {'name'} not documented + numpydoc/tests/hooks/example_module.py:11: GL08 The object does not have a docstring + numpydoc/tests/hooks/example_module.py:17: PR01 Parameters {'**kwargs'} not documented numpydoc/tests/hooks/example_module.py:17: PR07 Parameter "*args" has no description diff --git a/numpydoc/validate.py b/numpydoc/validate.py index 6a991a02..f940c418 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -660,24 +660,20 @@ def validate(obj_name, validator_cls=None, **validator_kwargs): # cls = Validator._load_obj(f"{doc.name[:-9]}.{cls_name}") ## Alternative cls_doc = Validator(get_doc_object(cls)) elif isinstance(doc, AstValidator): # Supports class traversal for ASTs. - names = doc._name.split(".") - - if len(names) > 2: # i.e. module.class.__init__ - nested_cls_names = names[1:-1] # class1,class2 etc. - cls_name = names[-2] - filename = doc.source_file_name # from the AstValidator object - - # Use AST to find the class node... - with open(filename) as file: - module_node = ast.parse(file.read(), filename) - - # Recursively find each subclass from the module node. - next_node = module_node - for name in nested_cls_names: - next_node = _find_class_node(next_node, name) - # Get the documentation. + ancestry = doc.ancestry + if len(ancestry) > 2: # e.g. module.class.__init__ + parent = doc.ancestry[-1] # Get the parent + cls_name = ".".join( + [ + getattr(node, "name", node.__module__) + for node in doc.ancestry + ] + ) cls_doc = AstValidator( - ast_node=next_node, filename=filename, obj_name=cls_name + ast_node=parent, + filename=doc.source_file_name, + obj_name=cls_name, + ancestry=doc.ancestry[:-1], ) else: # Ignore edge case: __init__ functions that don't belong to a class. From 39544d27961907930b528529dad6c35c3998b83d Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Tue, 24 Jun 2025 00:59:22 +1000 Subject: [PATCH 09/12] ci(test.yml): Added file existance check for hook tests Workflows do not seem to be running hook pytests. --- .github/workflows/test.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cd605d7f..5d263bda 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -99,6 +99,11 @@ jobs: python -m pip install . --pre --group test --group doc pip list + - name: "Check File Existence" + uses: thebinaryfelix/check-file-existence-action@v1 + with: + files: numpydoc/tests/hooks/test_validate_hook.py + - name: Run test suite run: | pytest -v --pyargs numpydoc From c14b2e88b1ab7d9638d02a56a6a0319366ec71fc Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Tue, 24 Jun 2025 01:05:27 +1000 Subject: [PATCH 10/12] ci(test.yml): Correct the workflow task name/version --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5d263bda..b104c092 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -99,8 +99,8 @@ jobs: python -m pip install . --pre --group test --group doc pip list - - name: "Check File Existence" - uses: thebinaryfelix/check-file-existence-action@v1 + - name: Check File Existence + uses: thebinaryfelix/check-file-existence-action@1.0.0 with: files: numpydoc/tests/hooks/test_validate_hook.py From 48f89744a5cfd62644fe4bac5189b9b79373e243 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Tue, 24 Jun 2025 01:16:05 +1000 Subject: [PATCH 11/12] ci(test.yml): Added explicit pytest call to the hooks directory --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b104c092..88ea4651 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -107,6 +107,7 @@ jobs: - name: Run test suite run: | pytest -v --pyargs numpydoc + pytest -v --pyargs numpydoc/tests/hooks - name: Test coverage run: | From c2d16fa6d70c64d8d4d2e6c378d48ee1eeda20b2 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Tue, 24 Jun 2025 01:20:05 +1000 Subject: [PATCH 12/12] ci(test.yml): Removed file existance test, after explicit call to hooks verifies execution --- .github/workflows/test.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 88ea4651..505ce2f3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -99,11 +99,6 @@ jobs: python -m pip install . --pre --group test --group doc pip list - - name: Check File Existence - uses: thebinaryfelix/check-file-existence-action@1.0.0 - with: - files: numpydoc/tests/hooks/test_validate_hook.py - - name: Run test suite run: | pytest -v --pyargs numpydoc