Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E126 hanging indent is too strict #167

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

E126 hanging indent is too strict #167

methane opened this issue Feb 17, 2013 · 17 comments
Labels

Comments

@methane
Copy link
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
Copy link
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.

@methane
Copy link
Contributor Author

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
Copy link
Member

@methane an alternate way of writing that is:

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

@myint
Copy link
Member

myint commented Mar 16, 2013

@methane, you have one too many indents.

from os import (
    path,
    listdir,
)

@methane
Copy link
Contributor Author

methane commented Mar 17, 2013

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

@florentx
Copy link
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.

@methane
Copy link
Contributor Author

methane commented Mar 21, 2013

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

@florentx
Copy link
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.

@methane
Copy link
Contributor Author

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
Copy link

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
Copy link
Member

myint commented Apr 5, 2013

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

@basepi
Copy link

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
Copy link
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
Copy link

basepi commented Apr 5, 2013

Cool! Thanks for the quick reply.

@methane
Copy link
Contributor Author

methane commented Apr 5, 2013

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

@methane methane closed this as completed Apr 5, 2013
@gotgenes
Copy link

@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
Copy link
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.

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

No branches or pull requests

6 participants