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

Fix D401 and D404 checks not working for docstrings containing only one word and ending with non-alpha character (#421) #422

Merged
merged 5 commits into from
Nov 30, 2019

Conversation

Strus
Copy link
Contributor

@Strus Strus commented Oct 18, 2019

No description provided.

def strip_non_alphanumeric(string: str) -> str:
"""Strip string from any non-alphanumeric characters."""
pattern = re.compile(r'[\W_]+')
Copy link
Member

@samj1912 samj1912 Nov 1, 2019

Choose a reason for hiding this comment

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

It doesn't make sense to do the compilation inside the function definition. You won't benefit from doing a compile. I would move the compile outside, so that it's done only once.

Copy link
Contributor Author

@Strus Strus Nov 22, 2019

Choose a reason for hiding this comment

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

Done. Sorry for long response time, I was on vacation.

@Nurdok
Copy link
Member

Nurdok commented Nov 22, 2019

To be honest, I'm not sure I like this change. The first line of a docstring should be a proper sentence that describes the function / class, and I don't see how one word, imperative of not, can qualify as a meaningful description. Happy to hear your thoughts on the subject. Perhaps we can add an issue that the first line should be > 1 word anyway?

@Strus
Copy link
Contributor Author

Strus commented Nov 22, 2019

Without my changes if you add any non-alpha character after the first word, like comma, D401 and D404 will also not work properly, so my fix should be added anyway, even if you decide that one-word documentations should not be considered vaild.

@Nurdok Nurdok merged commit d3ea9a0 into PyCQA:master Nov 30, 2019
@Nurdok
Copy link
Member

Nurdok commented Nov 30, 2019

Good point, merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants