Add config option and logic to allow the use of PEP257-style inline class attribute documentation

[mpyuglgwkxcmodyrpo]

PEP-257 is kind of vague when it comes to class vs. instance variable docs:

String literals occurring elsewhere in Python code may also act as documentation. They are not recognized by the Python bytecode compiler and are not accessible as runtime object attributes (i.e. not assigned to doc), but two types of extra docstrings may be extracted by software tools:

1. String literals occurring immediately after a simple assignment at the top level of a module, class, or init method are called “attribute docstrings”.
2. String literals occurring immediately after another docstring are called “additional docstrings”.

Please see PEP 258, “Docutils Design Specification”, for a detailed description of attribute and additional docstrings.

The way I interpret it is that it does allow a string after both class attributes AND instance attributes, especially since a class attribute is "a simple assignment". Additionally, but less relevant, the reference to PEP 258 seems strange because it was rejected entirely.

This PR adds support for inline class variable documentation.

The logic uses the class visitor to walk the class AST and check for constant exprs (ast.Expr -> ast.Constant) after ast.AnnAssign or ast.Assign directives. It will insert the attribute at the correct index in the documented args list so that it does not throw an out of order error. If attributes are declared in the class, they supersede the one declared inline.

I have additionally included tests, command line options, and docs in this PR. Let me know what you think of the overall idea and if the parameter name makes sense. I am also not confident that the added logic is in the correct function, but it does function properly where it is. I can also split it out into another helper, if you think that would make the code cleaner.

Thanks for the awesome project, it has already caught a number of doc inconsistencies with our code base.

[mpyuglgwkxcmodyrpo]

Fixing lints, will push again in a few.

fixed, will work on your other comments.

[jsh9]

Hi @mpyuglgwkxcmodyrpo , thanks for contributing to pydoclint!

I wasn't familiar with this type of style before, but recently I did see such styles being produced by various AI coding assistants I used. I think pydoclint should accommodate for such style.

Small note: when you push new commits, this PR's Github workflow doesn't run unless I click "approve" on my end. (I don't know now to configure this.) Therefore, you can run tox in your machine's pydoclint py env to make sure the Github workflow passes (at least for your OS and your py version).

[mpyuglgwkxcmodyrpo]

Pushed changes. I think the concept of "inline docstrings" is really only present in sphinx... but autodoc supports it for all three styles... see example numpy documentation and example google documentation on the sphinx website. The examples include BOTH the post-definition constant string AND #: either on the line before the attribute declaration or even on the same line as it.

One last thing I have to do is pull the documented type from the inline docstring, because apparently its valid to specify the type as the first thing in the comment and then follow it with a colon. I'll need to update the tests for that.
Done.

I'm thinking of changing the option from allow-inline-classvar-docs to allow-inline-class-attribute-docs. I am also wondering if we should throw a doc mismatch error IF arg-type-hints-in-docstring and allow-inline-classvar-docs are both True, the attribute type is specified in both the class header docstring and the inline docstring, AND they mismatch. Right now the inline docstring type is ignored entirely if they are both specified and both options are True. Thoughts?

I didn't implement the dedent bit because I needed the sources in multiple places. I could do an itertools.product, but unless you feel strongly about it I will not.

Also, I separated out the code that gathers the documented and actual attributes from the ast.ClassDef node, as I would have needed to gather them anyway to test the inline docs function. I can split it differently if you'd like.

Let me know if you want more changes.

[jsh9]

Hi @mpyuglgwkxcmodyrpo , I've been a bit busy these past few days. Let me try to get to it this weekend.

[mpyuglgwkxcmodyrpo]

No worries at all! I fixed the markdown formatting just now.

[jsh9]

Are there any safeguards when arg already exists in self.infoList and self.lookup?

[jsh9]

I think I'd like pydoclint to have this behavior:

• If True, users can only document class attributes inline. If there are any documentation of class attributes in the class docstring, report a violation. (We'd need to add a new violation code.)
• If False, users can only document class attributes in the class docstring. Inline documentations will result in a violation. (We'd need to add another new violation code.)

This is because pydoclint's philosophy is to promote consistent style within the same project. Project authors need to pick one and stick to it. If we allow users to document class attributes in both places (and only the class docstring, if present, is considered), it would not only promote ambiguity but also lead to communication overhead (project maintainers don't know why pydoclint only checks one place but not the other, and come to post an Issue).

For your reference, another config option --allow-init-docstring has the same logic:

• If True, __init__()'s input args can only be documented in __init__()'s docstring. (DOC305 will be reported)
• If False, __init__() cannot have a docstring. (DOC301 will be reported)

[mpyuglgwkxcmodyrpo]

I think consistency is the best policy. Though I'm thinking aicvd set to True should mean that inline classvar docs are required and that attributes documented in the class docstring are an error (new code). For aicvd set to False (the default), I think inline classvar docs should be ignored instead of throwing an error if both class docstring and inline docstrings are included. Inline docstrings could add extra context, but just the class level one should be required.

Thoughts on this? I'm currently implementing the feature as you requested above, for now.

[jsh9]

If we allow ignoring inline docstring (like the example below, where two type hints are inconsistent), we'd essentially be overlooking (if not condoning) ambiguous behaviors:

class MyClass:
    """
    My Class

    Attributes
    ----------
    data : list[float]
        The correct data
    """
    data: dict[str, int]
    """dict[str, int]: The incorrect data"""

[mpyuglgwkxcmodyrpo]

Fair enough. Implemented as requested. I still need to finish the tests you requested, but I am almost done.

[jsh9]

Could you add a test for this function in test_visitor_helper.py? Thanks!

[jsh9]

Could you add a test for this function in test_visitor_helper.py? Thanks!

[mpyuglgwkxcmodyrpo]

I will have time to work on this tomorrow, I think. Thanks for the feedback I look forward to implementing it so we can get this merged.