From d8f05e6bc6f31bea0229b4b3c0c18ceedb3f3217 Mon Sep 17 00:00:00 2001 From: Matt Gebert Date: Sun, 6 Apr 2025 00:27:51 +1100 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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: |