E126 hanging indent is too strict #167

Closed
methane opened this Issue Feb 17, 2013 · 17 comments

Comments

Projects
None yet
6 participants
@methane
Contributor

methane commented Feb 17, 2013

from os import(
        path,
        listdir)

This cause "E126 continuation line over-indented for hanging indent".
But below code is OK. from ... import ( should be handled like that.

# More indentation included to distinguish this from the rest.
def long_function_name(
        var_one, var_two, var_three,
        var_four):
    print(var_one)
@florentx

This comment has been minimized.

Show comment Hide comment
@florentx

florentx Feb 17, 2013

Contributor

Thank you for your feedback.

AFAICT, the first example does not trigger any warning with the pep8 tool.

For the second example, this was a deliberate choice of the E12 checks on issue #64 (@samv proposal). The PEP-8 itself advocates for "Use 4 spaces per indentation level." without excluding continuation lines indentation from this rule.
The spirit of the E12 check is to enforce PEP-8 for continuation lines, and to avoid inconsistent indentations through the code.

Of course each check can be ignored with --ignore E12 for example.

Contributor

florentx commented Feb 17, 2013

Thank you for your feedback.

AFAICT, the first example does not trigger any warning with the pep8 tool.

For the second example, this was a deliberate choice of the E12 checks on issue #64 (@samv proposal). The PEP-8 itself advocates for "Use 4 spaces per indentation level." without excluding continuation lines indentation from this rule.
The spirit of the E12 check is to enforce PEP-8 for continuation lines, and to avoid inconsistent indentations through the code.

Of course each check can be ignored with --ignore E12 for example.

@methane

This comment has been minimized.

Show comment Hide comment
@methane

methane Feb 18, 2013

Contributor

I'm sorry about wrong first example.
What I want to say is:

from os import(
        path,
        listdir,
        )

This cause "E126 continuation line over-indented for hanging indent" and
"E123 closing bracket does not match indentation of opening bracket's line".

Contributor

methane commented Feb 18, 2013

I'm sorry about wrong first example.
What I want to say is:

from os import(
        path,
        listdir,
        )

This cause "E126 continuation line over-indented for hanging indent" and
"E123 closing bracket does not match indentation of opening bracket's line".

@sigmavirus24

This comment has been minimized.

Show comment Hide comment
@sigmavirus24

sigmavirus24 Feb 18, 2013

Member

@methane an alternate way of writing that is:

from os import (path, foo, bar, bogus, #etc
                listdir, unlink, #etc,
                )
Member

sigmavirus24 commented Feb 18, 2013

@methane an alternate way of writing that is:

from os import (path, foo, bar, bogus, #etc
                listdir, unlink, #etc,
                )
@myint

This comment has been minimized.

Show comment Hide comment
@myint

myint Mar 16, 2013

Member

@methane, you have one too many indents.

from os import (
    path,
    listdir,
)
Member

myint commented Mar 16, 2013

@methane, you have one too many indents.

from os import (
    path,
    listdir,
)
@methane

This comment has been minimized.

Show comment Hide comment
@methane

methane Mar 17, 2013

Contributor

@myint That rule isn't applied to hanging indent.

Contributor

methane commented Mar 17, 2013

@myint That rule isn't applied to hanging indent.

@florentx

This comment has been minimized.

Show comment Hide comment
@florentx

florentx Mar 21, 2013

Contributor

Well, the PEP 8 recommends "Use 4 spaces per indentation level."
And there's no objective reason to apply a different rule for continuation line indentation.

The purpose of these E12* rules is to conform to PEP 8 and enforce some consistency for the indentation.

Contributor

florentx commented Mar 21, 2013

Well, the PEP 8 recommends "Use 4 spaces per indentation level."
And there's no objective reason to apply a different rule for continuation line indentation.

The purpose of these E12* rules is to conform to PEP 8 and enforce some consistency for the indentation.

@methane

This comment has been minimized.

Show comment Hide comment
@methane

methane Mar 21, 2013

Contributor

"per level" means indent for block.
It's not for continuation line.

Contributor

methane commented Mar 21, 2013

"per level" means indent for block.
It's not for continuation line.

@florentx

This comment has been minimized.

Show comment Hide comment
@florentx

florentx Mar 22, 2013

Contributor

"per level" means indent for block.
It's not for continuation line.

This is your interpretation of PEP 8.
If you want to be creative about your indentation, you can disable the E12 checks in the configuration.

Contributor

florentx commented Mar 22, 2013

"per level" means indent for block.
It's not for continuation line.

This is your interpretation of PEP 8.
If you want to be creative about your indentation, you can disable the E12 checks in the configuration.

@methane

This comment has been minimized.

Show comment Hide comment
@methane

methane Mar 22, 2013

Contributor

This is your interpretation of PEP 8.

pep8 says:

there should be no arguments on the first line and further indentation should be used to clearly distinguish itself as a continuation line.
...
Optional:

# Extra indentation is not necessary.
foo = long_function_name(
  var_one, var_two,
  var_three, var_four)

http://www.python.org/dev/peps/pep-0008/#indentation

I believe no one can interpret this to "hanging indent should be 1-level 4-space indent."

If you want to be creative about your indentation, you can disable the E12 checks in the configuration.

Yes, I already disabled it. But I think too strict default hurts new users.

Contributor

methane commented Mar 22, 2013

This is your interpretation of PEP 8.

pep8 says:

there should be no arguments on the first line and further indentation should be used to clearly distinguish itself as a continuation line.
...
Optional:

# Extra indentation is not necessary.
foo = long_function_name(
  var_one, var_two,
  var_three, var_four)

http://www.python.org/dev/peps/pep-0008/#indentation

I believe no one can interpret this to "hanging indent should be 1-level 4-space indent."

If you want to be creative about your indentation, you can disable the E12 checks in the configuration.

Yes, I already disabled it. But I think too strict default hurts new users.

@basepi

This comment has been minimized.

Show comment Hide comment
@basepi

basepi Apr 5, 2013

I have a similar "too strict" issue with E126.

cmd = 'git archive{prefix}{fmt} -o {output} {rev}'.format(
        rev = rev,
        output = output,
        fmt = ' --format={0}'.format(fmt) if fmt else '',
        prefix = ' --prefix="{0}"'.format(prefix if prefix else basename)
)

triggers E126, but I think this should be allowed to distinguish from an actual new indent level. Pulling the last ) onto the line above makes no difference.

basepi commented Apr 5, 2013

I have a similar "too strict" issue with E126.

cmd = 'git archive{prefix}{fmt} -o {output} {rev}'.format(
        rev = rev,
        output = output,
        fmt = ' --format={0}'.format(fmt) if fmt else '',
        prefix = ' --prefix="{0}"'.format(prefix if prefix else basename)
)

triggers E126, but I think this should be allowed to distinguish from an actual new indent level. Pulling the last ) onto the line above makes no difference.

@myint

This comment has been minimized.

Show comment Hide comment
@myint

myint Apr 5, 2013

Member

@basepi, --ignore=E126. (The whole point of E126 is to complain about indentation greater than 4 spaces.)

Member

myint commented Apr 5, 2013

@basepi, --ignore=E126. (The whole point of E126 is to complain about indentation greater than 4 spaces.)

@basepi

This comment has been minimized.

Show comment Hide comment
@basepi

basepi Apr 5, 2013

@myint Heh, thanks for this. I was just wondering if we wanted to try to bring it in line with PEP8. =)

basepi commented Apr 5, 2013

@myint Heh, thanks for this. I was just wondering if we wanted to try to bring it in line with PEP8. =)

@myint

This comment has been minimized.

Show comment Hide comment
@myint

myint Apr 5, 2013

Member

I think 4 is the most used convention in the Python code base. The following will show lines that end with ( and the line that immediately follows it.

locate .py | grep '\.py$' | grep python3.3 | xargs grep -A1 '($'

So in terms of following a convention, I think E126 is the way to go. If one prefers 8 instead 4, then I think --ignore=E126 is probably the best option.

Member

myint commented Apr 5, 2013

I think 4 is the most used convention in the Python code base. The following will show lines that end with ( and the line that immediately follows it.

locate .py | grep '\.py$' | grep python3.3 | xargs grep -A1 '($'

So in terms of following a convention, I think E126 is the way to go. If one prefers 8 instead 4, then I think --ignore=E126 is probably the best option.

@basepi

This comment has been minimized.

Show comment Hide comment
@basepi

basepi Apr 5, 2013

Cool! Thanks for the quick reply.

basepi commented Apr 5, 2013

Cool! Thanks for the quick reply.

@methane

This comment has been minimized.

Show comment Hide comment
@methane

methane Apr 5, 2013

Contributor

@myint Thank you. I agree E126 is not too strict for now.

Contributor

methane commented Apr 5, 2013

@myint Thank you. I agree E126 is not too strict for now.

@methane methane closed this Apr 5, 2013

@gotgenes

This comment has been minimized.

Show comment Hide comment
@gotgenes

gotgenes Jun 27, 2014

@methane said

I believe no one can interpret this to "hanging indent should be 1-level 4-space indent."

The 4aec4d180429 revision of PEP-8, updated 2014-05-20, explicitly aligns with @methane's assertion:

The 4-space rule is optional for continuation lines.

Optional:

# Hanging indents *may* be indented to other than 4 spaces.
foo = long_function_name(
  var_one, var_two,
  var_three, var_four)

pep8 needs to revise its hanging indent rules. The number of spaces within a hanging indent needs only to be consistent; 2 spaces, 4 spaces, 8 spaces – all are valid amounts of space for hanging indents.

@methane said

I believe no one can interpret this to "hanging indent should be 1-level 4-space indent."

The 4aec4d180429 revision of PEP-8, updated 2014-05-20, explicitly aligns with @methane's assertion:

The 4-space rule is optional for continuation lines.

Optional:

# Hanging indents *may* be indented to other than 4 spaces.
foo = long_function_name(
  var_one, var_two,
  var_three, var_four)

pep8 needs to revise its hanging indent rules. The number of spaces within a hanging indent needs only to be consistent; 2 spaces, 4 spaces, 8 spaces – all are valid amounts of space for hanging indents.

@sigmavirus24

This comment has been minimized.

Show comment Hide comment
@sigmavirus24

sigmavirus24 Jun 28, 2014

Member

@gotgenes I disagree that pep8 needs a fix because in the Yes section (which you helpfully omitted) it says:

# Hanging indents should add a level.
foo = long_function_name(
    var_one, var_two,
    var_three, var_four)

And the example level it uses is 4 spaces. In other words, pep8 is still correct, and the ability to turn off this check keeps with the PEP's understanding of optional.

Member

sigmavirus24 commented Jun 28, 2014

@gotgenes I disagree that pep8 needs a fix because in the Yes section (which you helpfully omitted) it says:

# Hanging indents should add a level.
foo = long_function_name(
    var_one, var_two,
    var_three, var_four)

And the example level it uses is 4 spaces. In other words, pep8 is still correct, and the ability to turn off this check keeps with the PEP's understanding of optional.

@fr33jc fr33jc referenced this issue in brantai/python-rightscale Feb 3, 2015

Merged

Honor the 'expires_in' value returned with the OAUTH token. #11

edmorley referenced this issue in edmorley/peep Mar 20, 2015

Fix flake8 warnings
$ flake8
peep.py:52:1: E302 expected 2 blank lines, found 1
peep.py:63:25: E113 unexpected indentation
peep.py:64:25: E113 unexpected indentation
peep.py:216:32: E262 inline comment should start with '# '
peep.py:392:141: E501 line too long (157 > 140 characters)
peep.py:542:5: E303 too many blank lines (2)
peep.py:703:17: E126 continuation line over-indented for hanging indent
peep.py:785:17: E126 continuation line over-indented for hanging indent
setup.py:6:5: F401 'multiprocessing' imported but unused
tests/__init__.py:175:38: F841 local variable 'exc' is assigned to but never used
tests/__init__.py:314:12: E121 continuation line under-indented for hanging indent
tests/__init__.py:316:17: E126 continuation line over-indented for hanging indent
tests/__init__.py:337:21: E126 continuation line over-indented for hanging indent

@tammoippen tammoippen referenced this issue in nest/nest-simulator Apr 11, 2016

Merged

TravisCI: Check for PEP8 #298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment