diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b463ee42..505ce2f3 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: | @@ -95,12 +96,13 @@ jobs: - name: Install run: | - python -m pip install . --group test --group doc + python -m pip install . --pre --group test --group doc pip list - name: Run test suite run: | - pytest -v --pyargs . + pytest -v --pyargs numpydoc + pytest -v --pyargs numpydoc/tests/hooks - name: Test coverage run: | 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.""" 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_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 b235313b..0a2177c1 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 @@ -61,8 +62,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 @@ -88,8 +105,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 +151,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 +186,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 +227,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,7 +260,7 @@ 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 diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index 9f0f7942..5a66885a 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -1,5 +1,6 @@ import warnings from contextlib import nullcontext +from dataclasses import dataclass from functools import cached_property, partial, wraps from inspect import getsourcefile, getsourcelines @@ -1305,6 +1306,109 @@ 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 + + +@dataclass +class DataclassWithDocstring: + """ + A class decorated by `dataclass`. + + To check the functionality of `dataclass` objects do not break the Validator. + As param1 is not documented this class should also raise PR01. + """ + + param1: int + + +class ClassWithPropertyObject: + """ + A class with a `property`. + + To check the functionality of `property` objects do not break the Validator. + + Parameters + ---------- + param1 : int + Description of param1. + """ + + def __init__(self, param1: int) -> None: + self._param1 = param1 + + @property + def param1(self) -> int: + """ + Get the value of param1. + + Returns + ------- + int + The value of param1. + """ + return self._param1 + + class TestValidator: def _import_path(self, klass=None, func=None): """ @@ -1660,6 +1764,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 +1793,39 @@ 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 test_dataclass_object(self): + # Test validator methods complete execution on dataclass objects and methods + # Test case ought to be removed if dataclass objects properly supported. + result = validate_one(self._import_path(klass="DataclassWithDocstring")) + # Check codes match as expected for dataclass objects. + errs = ["ES01", "SA01", "EX01", "PR01"] + for error in result["errors"]: + assert error[0] in errs + errs.remove(error[0]) + + # Test initialisation method (usually undocumented in dataclass) raises any errors. + init_fn = self._import_path(klass="DataclassWithDocstring", func="__init__") + result = validate_one(init_fn) + # Check that __init__ raises GL08 when the class docstring doesn't document params. + assert result["errors"][0][0] == "GL08" + + def test_property_object(self): + # Test validator methods complete execution on class property objects + # Test case ought to be removed if property objects properly supported. + result = validate_one( + self._import_path(klass="ClassWithPropertyObject", func="param1") + ) + # Check codes match as expected for property objects. + errs = ["ES01", "SA01", "EX01"] + for error in result["errors"]: + assert error[0] in errs + errs.remove(error[0]) + def decorator(x): """Test decorator.""" diff --git a/numpydoc/validate.py b/numpydoc/validate.py index d0debfa2..a1b9ad7e 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -572,6 +572,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. @@ -639,20 +647,49 @@ 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 = doc.code_obj.__qualname__.split(".")[0] - 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.is_function_or_method and doc.name.endswith(".__init__"): + # Import here at runtime to avoid circular import as + # AstValidator is a subclass of Validator class without `doc_obj` attribute. + from numpydoc.hooks.validate_docstrings import ( + AstValidator, # Support abstract syntax tree hook. + ) + + if hasattr(doc, "code_obj"): # All Validator objects have this attr. + 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. + 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=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. + 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: