Code

Opened 6 months ago

Closed 6 months ago

#21288 closed Cleanup/optimization (fixed)

pep8 cleanup: continuation line over-indented for hanging indent

Reported by: timo Owned by: nobody
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

To find errors, ensure you have an up-to-date master with the flake8 config in setup.cfg, then remove E126 from the ignore list there. You can then run flake8 from the directory with setup.cfg in it to list all errors - there should be about 300.

Attaching an example patch to get you started.

Apply it, if you'd like:

patch -p1 -i E126.diff

Attachments (2)

E126.diff (5.4 KB) - added by timo 6 months ago.
generic-E126.diff (1.6 KB) - added by alasdair 6 months ago.
PEP8 cleanup of contrib.contenttypes.generic.py

Download all attachments as: .zip

Change History (6)

Changed 6 months ago by timo

Changed 6 months ago by alasdair

PEP8 cleanup of contrib.contenttypes.generic.py

comment:1 Changed 6 months ago by alasdair

This cleanup is a bit less straight forward than the other recent pep8 tickets.

The attached patch makes sense to me, when we are changing the indentation of homogeneous list/dictionary items and function arguments.

However other E126 errors seem trickier to fix.

I've included a patch for django/contrib/contenttypes/generic.py as an example. I'm concerned that I've made the code less readable, because the indentation to distinguish between dictionary keys and values has been removed.

Another tricky example is lines 113 & 114 of django.db.models.base.py. It's not clear to me how to re-format the code to remove the error.

comment:2 Changed 6 months ago by timo

Thanks for bring this up. For the contenttypes, I would put the key/value pairs on the same line, even if we have to go a bit over 80 chars. 80 chars is a soft guideline that may be broken for readability.

Similarly for db.models.base I think tuple(x.DoesNotExist for x in parents if hasattr(x, '_meta') and not x._meta.abstract) or (ObjectDoesNotExist,), could all be on the same line. Feel free to make adjustments if you feel otherwise.

comment:3 Changed 6 months ago by alasdair

Thanks for the suggestions. They were enough for me to finish my patch.

Pull request https://github.com/django/django/pull/1780

I've left inline comments on the pull request about a few of the changes I wasn't 100% sure about. Feel free to change before committing.

comment:4 Changed 6 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In b289fcf1bfeaa717ed465b2529a275b61dc02d92:

Fixed #21288 -- Fixed E126 pep8 warnings

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.