Opened 9 years ago

Last modified 9 years ago

#25263 closed Cleanup/optimization

How closely we should adhere to PEP 257 — at Version 6

Reported by: Samuel Bishop Owned by: nobody
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

Currently the documentation contained in Django's source code has many places where it could be improved, most significantly by having more of it. In order to help discover undocumented code, I ran PEP 257 on the entire project. The thousands of warnings were quite a shock and it was pretty obvious that a pull request for all of the changes would be too large.

It seemed like a good idea to open this ticket and find out if continuing to create new tickets and pull requests for more of this kind of documentation improvement is in line with the overall project code standards. I'm opening a pull request that contains a partial cleanup of the existing documentation inside 'django.views'. https://github.com/django/django/pull/5131

Change History (6)

comment:1 by Samuel Bishop, 9 years ago

Description: modified (diff)

comment:2 by Claude Paroz, 9 years ago

I think we have already many code quality checks with Flake8. Are those conventions really adding value or is this just a trend to adhere to more conventions? Frankly, the important thing for me is to have documentation. Then if we add newlines or not for single line comments and such things look more and more like bikeshedding for me. </rant>

comment:3 by Anssi Kääriäinen, 9 years ago

I agree with claudep. The idea is that you have clean and well structured code so that it is easy to maintain. But there is a point after which the maintenance burden of all the rules is more than what we get in return. At this point I think we shouldn't add more rules, at least not without considering carefully if the new rules really give us enough in return.

comment:4 by Tim Graham, 9 years ago

Resolution: wontfix
Status: newclosed

I agree with what has already been said.

comment:5 by Samuel Bishop, 9 years ago

Resolution: wontfix
Status: closednew

I'm reopening it since I didn't get a chance to comment on the criticism before it was closed.
It's worth considering that some of the things that PEP 257 recommends are areas beneficial to the level of documentation in the source code.

Checks D100, D101, D102, and D103 all indicate Modules, Classes and Methods that have no documentation at all.
Others are mainly about consistency and some of them are definitely of minimal improvement or possibly even detrimental. D206 overlaps with PEP8

The full list is below.

Missing Docstrings
D100 Missing docstring in public module
D101 Missing docstring in public class
D102 Missing docstring in public method
D103 Missing docstring in public function
Whitespace Issues
D200 One-line docstring should fit on one line with quotes
D201 No blank lines allowed before function docstring
D202 No blank lines allowed after function docstring
D203 1 blank line required before class docstring
D204 1 blank line required after class docstring
D205 1 blank line required between summary line and description
D206 Docstring should be indented with spaces, not tabs
D207 Docstring is under-indented
D208 Docstring is over-indented
D209 Multi-line docstring closing quotes should be on a separate line
D210 No whitespaces allowed surrounding docstring text
Quotes Issues
D300 Use “”“triple double quotes”“”
D301 Use r”“” if any backslashes in a docstring
D302 Use u”“” for Unicode docstrings
Docstring Content Issues
D400 First line should end with a period
D401 First line should be in imperative mood
D402 First line should not be the function’s “signature”

In my own mind, this subset feels like its worth adopting.
Other than the ones for missing docstrings, most of Django's documentation already fits within these checks.
D100 Missing docstring in public module
D101 Missing docstring in public class
D102 Missing docstring in public method
D103 Missing docstring in public function
D201 No blank lines allowed before function docstring
D202 No blank lines allowed after function docstring
D203 1 blank line required before class docstring
D204 1 blank line required after class docstring
D209 Multi-line docstring closing quotes should be on a separate line
D300 Use “”“triple double quotes”“”
D401 First line should be in imperative mood
D402 First line should not be the function’s “signature”

Last edited 9 years ago by Samuel Bishop (previous) (diff)

comment:6 by Tim Graham, 9 years ago

Description: modified (diff)

Feel free to raise the idea on the DevelopersMailingList. Personally, I don't think requiring docstrings for everything is needed and I don't care for whitespace around class docstrings.

Note: See TracTickets for help on using tickets.
Back to Top