Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#14854 closed Uncategorized (wontfix)

Follow PEP 8 -- Style Guide for Python Code

Reported by: sorl Owned by: nobody
Component: Core (Other) Version: 1.2
Severity: Normal Keywords: pep8
Cc: mikko@…, eallik@…, pbagwl@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

There is currently a lot of Django code that violates the PEP 8 style guide with respect to "Maximum Line Length - Limit all lines to a maximum of 79 characters.". I don't think this is a problem for "old" code and there is no need to go fix it unless you have got nothing better to do. How ever for recent commits there is no valid reason not to follow this recommendation from my standpoint. It is almost as if this has grown into an own django-long-lines cult, people will look at Django code and copy these bad long-lines examples. In particular I was looking at the Class Based Generic views which is a recent commit and it contains quite a few lines longer than 79 characters.

Attachments (0)

Change History (6)

comment:1 Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

Closing wontfix, for two reasons.

Firstly, the opening comments of PEP8 has a whole section entitled "A foolish consistency is the hobgoblin of little minds". This ticket is exactly the kind of prescriptive nonsense that that comment was directed at. Overall readability is more important than concrete adherence to a rulebook. Sometimes the 79 character limit is an impediment to readability. When this happens is entirely a matter of judgement, but pointing at PEP8 as a limit that must be enforced at all cost completely misses the point of PEP8.

Secondly, and more importantly, there is no course of action for this ticket -- something that the ticket description itself acknowledges.

comment:2 Changed 3 years ago by sorl

Sure, I'd argue that readability can be good with longer lines but most of the time its not and its a sign that you should write it a bit differently. For example this code is not better off using long lines in my mind:

https://github.com/django/django/blob/master/django/views/generic/date_based.py#L269

For what its worth I think grok code looks really nice as a comparison: http://svn.zope.org/repos/main/grok/trunk/src/grok/

comment:3 Changed 3 years ago by anonymous

Ok so finally for some metrics.
Lines of code was collected by issuing (at top level of distribution packages):

    find -name "*.py" | xargs wc -l

Lines that have more than 79 characters were collected issuing (at top level of distribution packages):

   find -name "*.py" | xargs awk '{ if (length($0) > 79) printf("%s:%s\n", FILENAME, FNR);}' | wc -l

Results:

What LOCS Lines > 79 chars Ratio
Django trunk 101106 4525 4.48%
Turbo Gears dev 8781 296 3.37%
Pylons latest 10378 218 2.10%
Zope 3.4.0c1 269273 4626 1.72%
Twisted 10.2.0 269535 2946 1.09%
Grok trunk 7352 26 0.35%

comment:4 Changed 3 years ago by RaceCondition

  • Cc eallik@… added

Does Django have any code style guidelines whatsoever?

comment:5 Changed 3 years ago by bpeschier

0-brain-involved-look-in-docs: http://docs.djangoproject.com/en/dev/internals/contributing/#coding-style

This ticket is only going somewhere if people interested will create a patch for it for fun. Sure PEP8 is preferred and if it impedes readability the writer of a patch will be told to follow PEP8, but remember you cannot provide a patch, close the ticket and all will be fine; it will come back time and time again...

It this were a valid ticket, I would love to see a regressiontest for PEP8 because I believe it would have to be sentient to follow its spirit O:-)

But in all fairness: post patches (not just file tickets) for specific pieces of code you think are really unreadable and they have no reason not to be committed if they are just.

comment:6 Changed 3 years ago by pbagwl

  • Cc pbagwl@… added
  • Severity set to Normal
  • Type set to Uncategorized

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.