Add config option and logic to allow the use of PEP257-style inline class attribute documentation#278
Conversation
|
fixed, will work on your other comments. |
…lass attribute documentation
403da31 to
5915074
Compare
|
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 |
|
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
I'm thinking of changing the option from I didn't implement the Also, I separated out the code that gathers the documented and actual attributes from the Let me know if you want more changes. |
|
Hi @mpyuglgwkxcmodyrpo , I've been a bit busy these past few days. Let me try to get to it this weekend. |
96a1196 to
178db4a
Compare
|
No worries at all! I fixed the markdown formatting just now. |
| ) -> None: | ||
| """Insert an Arg at a specific index.""" | ||
| self.infoList.insert(index, arg) | ||
| self.lookup[arg.name] = arg.typeHint |
There was a problem hiding this comment.
Are there any safeguards when arg already exists in self.infoList and self.lookup?
There was a problem hiding this comment.
I added a check and exception. Thoughts? It should be an edge case.
|
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. |
|
Sorry this got away from me with the holidays and some work deadlines. Still on my radar. |
478231d to
512daf6
Compare
|
I thought my tox checks were passing and then I must have changed something. All good now! |
|
Not sure what is going on here, I cannot reproduce the reformatting changes locally (muff format is failing in tox pre-commit step). The version matches the one in the pre-commit config. |
PEP-257 is kind of vague when it comes to class vs. instance variable docs:
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) afterast.AnnAssignorast.Assigndirectives. 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.