Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Exempt properties from D401 #546

Merged
merged 15 commits into from Aug 15, 2021
3 changes: 3 additions & 0 deletions docs/release_notes.rst
Expand Up @@ -8,6 +8,9 @@ Release Notes
Current Development Version
---------------------------

Bug Fixes

* Properties no longer cause D401
samj1912 marked this conversation as resolved.
Show resolved Hide resolved

6.1.1 - May 17th, 2021
---------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/pydocstyle/checker.py
Expand Up @@ -500,7 +500,7 @@ def check_imperative_mood(self, function, docstring): # def context
"Returns the pathname ...".

"""
if docstring and not function.is_test:
if docstring and not function.is_test and not function.is_property:
stripped = ast.literal_eval(docstring).strip()
if stripped:
first_word = strip_non_alphanumeric(stripped.split()[0])
Expand Down
8 changes: 8 additions & 0 deletions src/pydocstyle/parser.py
Expand Up @@ -218,6 +218,14 @@ def is_overload(self):
return True
return False

@property
def is_property(self):
"""Return True iff the method decorated with property."""
Copy link
Member

Choose a reason for hiding this comment

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

This says iff which tends to only mean "if and only if" but that's not actually true here because anything with the string "property" in the name will return True, not just the @property built-in decorator

for decorator in self.decorators:
Copy link
Member

Choose a reason for hiding this comment

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

I'm having a hard time understanding what the difference is between self.decorators and the argument decorators. Can these names be more descriptive?

Also couldn't this be something like

return any(decorator.name in decorators for decorator in self.decorators)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve improved this. I also changed the above is_overload to match.

if "property" in decorator.name:
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure if I create a decorator named myproperty this will disable D401 which is far from ideal. Why not if decorator.name == "property":?

return True
return False

@property
def is_test(self):
"""Return True if this function is a test function/method.
Expand Down
5 changes: 5 additions & 0 deletions src/tests/test_cases/test.py
Expand Up @@ -42,6 +42,11 @@ def overloaded_method(a):
"D418: Function/ Method decorated with @overload"
" shouldn't contain a docstring")

@property
def foo(self):
"""The foo of the thing, which isn't in imperitive mood."""
return "hello"

@expect('D102: Missing docstring in public method')
def __new__(self=None):
pass
Expand Down