Constructor checking for AST validator #622
Conversation
…ructor Introduced GL08 checking for constructors (i.e. __init__) but didn't traverse the parent class heireacy when importing the __init__ parent from a module.
…checking for initialisation docstring Test written due to bug missed in numpy#592.
…rings 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.
…/mattgebert/numpydoc into ConstructorChecking_ASTValidator
…ule tests for AST (AbstractSyntaxTree) discovery of constructor method parent class.
…ase installation, and changed pytest invocation to use `numpydoc` instead of `.`, for consistency with `test` job In response to discussion on numpy#591
… to match `tests\` directory
|
I've noted that even despite using All pytest unit testing, including Local machine collecting the pytest collection
|
… pytest execution
| 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 |
There was a problem hiding this comment.
I don't think this is the way to do this. The DocstringVisitor class is already traversing the tree and capturing node ancestry (currently, just the name, but after #623, it will be all AST node ancestors). This is then used to instantiate the AstValidator, so we should be able to use that information to modify the check for what you are trying to achieve. A lot of the logic here is pretty much doing what those classes are doing, but for a second time, which I would like to avoid.
You may be able to simply filter out any cases that shouldn't be flagged in DocstringVisitor._get_numpydoc_issues(), at which point you would have access to the AST node and its ancestry. This is how the ignore rules are applied for the hook.
There was a problem hiding this comment.
Brilliant - I was actually hoping this could be the case prior to making these changes.
Once #623 is merged I'll be happy update this logic and use the ancestry.
Modifies
validate.pyto include checks for constructors methods (class.__init__) implemented by class docstring when using the AST validator, which is essential for hook use (probably the majority use case for numpydoc). Also fixes nested class references (previously implemented incorrectly).This pull request now adds checks for the AST class instance, and if using the AST, splits the names of the submodule nested classes, and finds the parent through a AST node
walkprocedure, before allowing the cls_doc check for parameter mismatches.Solution to #591 (update from #592) and fixed inconsistency in #605.