Skip to content

Fix: docstring_parser/numpydoc.py maps both 'Parameters' and 'Attributes'... - #114

Closed
M001N wants to merge 2 commits into
rr-:masterfrom
M001N:oss-engine/2a9e4022-2baa9606
Closed

M001N wants to merge 2 commits into
rr-:masterfrom
M001N:oss-engine/2a9e4022-2baa9606

Conversation

@M001N

@M001N M001N commented Aug 17, 2026

Copy link
Copy Markdown

Summary

Changed attrdoc.add_attribute_docstrings() to build its dedup set from all DocstringParam items in docstring.meta (regardless of style-specific args[0] tag) instead of the narrower docstring.params property, so attributes already documented via a numpydoc Attributes section are correctly recognized as already-covered and not duplicated when merging in source-level attribute docstrings. (The Docstring.params filter itself in common.py was already fixed in commit b4100c8, prior to this session's work.)

Problem

rr-/docstring_parser issue reference: rr-/docstring_parser#21

Root Cause

docstring_parser/numpydoc.py maps both 'Parameters' and 'Attributes' headers to DocstringParam via ParamSection, distinguished only by item.args[0] ('param' vs 'attribute'). Docstring.params (common.py) filtered purely by isinstance(item, DocstringParam), so Attributes-section entries leaked into params for NUMPYDOC-style docstrings. This part was already fixed in a prior commit (b4100c8) by narrowing Docstring.params to exclude NUMPYDOC items tagged 'attribute'. However, that narrowing had an unaddressed side effect: attrdoc.py's add_attribute_docstrings() built its 'already documented' name set from docstring.params (now narrower for NUMPYDOC), so an attribute already covered by an explicit Attributes section was no longer recognized as documented, and a duplicate DocstringParam was appended from the source-level docstring parse.

Testing

PASS - full suite 256 passed, including the new regression test and the previously-added test_numpydoc.py::test_params_excludes_attributes and related tests.

Related Issue

rr-/docstring_parser#21

M001N added 2 commits August 17, 2026 11:38
Docstring.params is meant to list function/constructor parameters, but
for NUMPYDOC-style docstrings it also picked up entries from a NumPy-
style Attributes section (class attributes), since numpydoc.py tags
both kinds with the same DocstringParam type and only differs via
item.args[0] (param vs attribute).

Fix .params to exclude attribute-tagged items, but only when the
docstring style is NUMPYDOC. Other styles are left untouched:
Google-style intentionally folds its own Attributes section into
.params (see test_google.py::test_attributes) and the attribute-
docstring merging feature (attrdoc.py / parse_from_object) relies on
attribute-tagged DocstringParam items appearing in .params too, for any
style. Narrowing the filter unconditionally would have broken both.

Also update numpydoc.compose() to build its Parameters/Attributes/
Receives/Other Parameters sections from docstring.meta instead of the
now-narrower docstring.params, so composing a previously-parsed
docstring still round-trips its Attributes/Receives/Other Parameters
sections correctly.

Add regression tests in test_numpydoc.py covering the reporter's
Attributes-only reproducer and a docstring with both Parameters and
Attributes sections. Update the existing test_attributes to assert the
corrected .params behavior (verifying detailed field parsing via
docstring.meta instead).
Docstring.params now excludes NumpydocStyle attribute-tagged entries
(fix for rr-#21). attrdoc.add_attribute_docstrings() relied on
docstring.params to know which attribute names were already documented,
which meant an attribute already covered by an explicit numpydoc
Attributes section was no longer recognized as such, causing a duplicate
DocstringParam to be appended from the source-level docstring. Dedup now
checks all DocstringParam items in docstring.meta regardless of style tag.

Adds a regression test in test_parse_from_object.py reproducing
parse_from_object() on a class with both a numpydoc Attributes section
and a matching source-level attribute docstring.

@mauvilsa mauvilsa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking an interest in the project. I'm going to close this one, though. Looking at the volume and pattern of issues and PRs you opened around the same time, this appears to have been generated semi-automatically rather than written in response to a real need in this codebase, and the changes have problems I'd rather not spend review time itemizing. Contributions like that cost maintainers more than they give back, so they aren't welcome here. If I've misjudged this and you can explain the specific problem you were solving and why this is the right fix, reply here and I'll take another look.

@mauvilsa mauvilsa closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants