darglint icon indicating copy to clipboard operation
darglint copied to clipboard

Handling properties

Open vyasr opened this issue 5 years ago • 4 comments

I've been trying to apply darglint to my project, which uses Google style docstrings. However, I run into issues because my project makes extensive use of properties, and the docstrings for properties conform to different rules than functions. Following, for instance, the style shown in the Napoleon documentation (which I use to generate my Sphinx documentation), there is no "Returns" section, and the setter for the property does not document its argument. Is there a way around this that I'm missing in the API?

If not, from a quick look at the source, I think you would need to update the FunctionDescription class in function_description.py to recognize whether a function is a property, and you can get this from the ast by modifying _get_all_methods in function_description.py to check the decorator_list property of the node. However, the IntegrityChecker class would have to use this information to decide when missing these parts is an error and when it's not (if it's a property). Does that sound right?

Thanks for a great plugin!

vyasr avatar May 30 '20 05:05 vyasr

This seems pretty reasonable. This use case doesn't conform by the Google style guide, which says that properties should follow the same conventions as attributes (which are documented in the class docstring, something darglint doesn't handle yet.)

But since the Google style guide leaves the property without a docstring, if there is one, I guess it's reasonable to assume it may follow the Sphinx guide. It would be weird to require a Google docstring to conform to Sphinx expectations, but allowing it seems fine.

terrencepreilly avatar May 30 '20 19:05 terrencepreilly

You're right, this doesn't strictly conform to the Google style guide. However, I have found it in a few other places. Documenting properties in this manner has a couple of advantages IMO, because these docstrings are inherited by subclasses and they're more intuitive from a developer's perspective because they're next to the property and therefore less likely to get out of sync. The inheritance is particularly nice from a Sphinx perspective because autodoc can include inherited members this way. Thanks for being open to making the change!

vyasr avatar May 30 '20 20:05 vyasr

In the meantime, is there a way to turn off darglint checks for all properties? I don't want to manually # noqa each one, nor turn off darglint entirely...

steven-murray avatar Jul 13 '20 17:07 steven-murray

Does # noqa even work to workaround this? It doesn't seem to be doing anything no mater where it is put (after @property, after the function def, etc).