black: Black violates pep8 recommendation with long argument list

@JelleZijlstra Are you serious??? Reopen the issue right now or I am forking. This is a bug. Period. It does not comply with pep8. It does not comply with autopep8. It must be fixed.

Black isn’t going to change this style now because it would be too disruptive for our users.

Either this is fixed, or I am forking. Period.

You’re saying that like it’s a threat. This is open source software, maintained on github. There’s a fork button, it’s easy. That’s how OSS works. Feel free to fork Black and I honestly wish you the best of luck maintaining your fork.

You might have noticed that closing brackets are always dedented and that a trailing comma is always added. Such formatting produces smaller diffs; when you add or remove an element, it’s always just one line. Also, having the closing bracket dedented provides a clear delimiter between two distinct sections of the code that otherwise share the same indentation level (like the arguments list and the docstring in the example above).

This is our reasoning for choosing this style. We’re not convinced it’s a violation of PEP 8 - its point is to make sure there’s visual separation of the function arguments from the body. Dedenting the closing bracket is enough visual separation in our opinion - it looks like your opinion is different. That’s perfectly fine.

The backwards incompatible changes highlighted above are examples when people convinced the core maintainers of Black about a better formatting style, so that option is generally open. So far the arguments in this issue haven’t convinced any of us; if there are more - different - arguments, let them come. If not, please let it go and let’s move on with our lives.

To be honest, I much prefer the black style. It makes it a lot easier to read the parameters in cases where you have lots of them. Even in your first example I find the black approach to be preferable. So I would suggest the contrary, i.e. that the black style is maintained as is even if it is in breach of PEP8.

@zsol I consider it offensive to close a bug just because someone doesn’t feel like to fix it.

@JelleZijlstra

This is in your documentation

NOTE: This is a beta product Black is already successfully used by many projects, small and big. It also sports a decent test suite. However, it is still very new. Things will probably be wonky for a while. This is made explicit by the “Beta” trove classifier, as well as by the “b” in the version number. What this means for you is that until the formatter becomes stable, you should expect some formatting to change in the future. That being said, no drastic stylistic changes are planned, mostly responses to bug reports.

This is a bug.

@JelleZijlstra Are you serious??? Reopen the issue right now or I am forking

Dear @stefanoborini please tone it down. This forum is for civil discussions where the above style is not tolerated.

the bottom of the section you linked shows this:

or it may be lined up under the first character of the line that starts the multiline construct, as in:

my_list = [
    1, 2, 3,
    4, 5, 6,
]

result = some_function_that_takes_arguments(
    'a', 'b', 'c',
    'd', 'e', 'f',
)

which I believe is in line with what black is doing

@ovidiu-munteanu I have the completely opposite view. It makes it harder to read and discern what is parameters and what is code.

@mk-pmb not the point. If black wants to be opinionated, at least it should comply with pep8.

@asottile that’s for when you invoke a function, not for when you define it. When you invoke a function, the problem of visual collision between the indented arguments and the following blocks does not exist, because the code that follows is always at the same indentation level as the function call.

it’s not immediately following and the same indentation though, black always puts a paren on the next line so they aren’t immediately the same (and uses identical style for calls and definitions)

Thanks for taking the time to bring this issue to discuss.python.org, it definitely cleared things up. I think it’s time to change our readme to not say the Black style is a strict subset of pep8 anymore.

@zsol

We’re not convinced it’s a violation of PEP 8

From comment by Guido van Rossum:

I would prefer PEP 8 to explicitly discourage putting ): on a line by itself, since I find it ugly, Black notwithstanding.

Also, from comment by Brett Cannon:

The trick with style guides is the potential list of unacceptable formats is nearly infinite while documenting what is acceptable is tractable. So assume if something is not listed as acceptable then it isn’t.

I would like to express my support for @stefanoborini and option number (1) which is pep8 compliant, has the frowny face on a separate line, and visually separates the arguments from the function body.

guys, I don’t care, do what you want.

@zsol I say that because I cannot believe that you would ignore an explicit statement from pep8, and I don’t think that anybody wants two competing formatters that reformat the code one against each other. Because believe me, I won’t let this go.

The nice thing about strongly opinionated code formatters is, it’s so easy to make your own. 😉

To me, it seems a bad idea to indent something additional times. That is like having stairs that suddenly have a step twice the height and depth. You dont want to walk on stairs like this.

@stefanoborini , I never stated I was not wrong with regard to pep8 which I could not care less ( as pep8 is a recommendation, not a RFC MUST or SHOULD)

pep8 is a recommendation with a sensible reason behind it to enforce.

I care about the size of the diff produced by people in my team

This has nothing to do with the matter under discussion. The matter under discussion is the level of indentation of the arguments, which would have no effect on the size of the diff.

I say it again as the discussion made it clear it’s not gone through. I am not discussing the frowny face on a separate, isolated line, I am discussing the missing level of indentation of the arguments (and of the frowny face as a consequence)

so with this regard, black is playing a wonderful job, having the closing ): on a separate play for me the role of visual delimiter with the body, and help reduce diff-size when arguments are added. So the purpose of my comment was to express my support to black maintainers.

Black maintainers can keep the closing frown on a separate line, as long as it’s indented. I was never against the frowny face on a separate line.

but “wrong visually” is subjective (as stated previously) , end of the story.

Indentation matters, otherwise we would all still be coding in Fortran 77.

So I don’t think your comment is bringing anything to the discussion, you’ve already showed to everybody your holy book

The holy book is an official python recommendation and is enforced by all formatters except black.

Let’s even add a list of known differences to PEP-8.

@zsol

We’re not convinced it’s a violation of PEP 8

It’s literally written in the PEP and autopep8 does not format it that way. How can you argue that?

Dedenting the closing bracket is enough visual separation in our opinion - it looks like your opinion is different. That’s perfectly fine.

It’s not only my opinion. It’s in pep8. Literally, written in the pep8.

If not, please let it go and let’s move on with our lives.

Again, I am sorry but this is unacceptable. Either this is fixed, or I am forking. Period.

In all your previous examples I think you said that you wanted the formatting shown by my second example, no? Regardless, I think (1) is harder to read than Black’s, and (2) is much uglier in my opinion (but easier to read than 1). In all three cases, I think it is clear where the args are and where the body is. And since Black has already chosen one of the three options, and they all achieve the same goal, existing behaviour should win.

Re: formatters. So autopep8 is breaking something? I’m confused. Why wouldn’t different formatters with different styles reformat your code when you use them both together? Doesn’t seem like this is Black’s fault.

And if PyCharm’s linter is flagging this as incorrect, seems like it should be fixed there, as while this Black style does not conform to PEP8 (it’s not one of the suggested styles), it doesn’t violate it either (it’s not one of the “don’t do this” styles). And in my opinion a linter should be concerned with valid code, not PEP8 code. Focusing too much on PEP8 is a good way to miss the forest for the trees and ignore smellier parts of your code that are nevertheless “PEP8 compliant”.

@allan-simon

now that I’ve read more carefully PEP-8 I agree with you with ambiguity and I have created in that sense a issue on pep repo python/peps#1267

as the PEP-8 gives as acceptable this part of code

    # Add a comment, which will provide some distinction in editors
    # supporting syntax highlighting.
    if (this_is_one_thing and
        that_is_another_thing):
        # Since both conditions are true, we can frobnicate.
        do_something()

This is part of a larger set of examples: quoting

When the conditional part of an if-statement is long enough to require 
that it be written across multiple lines, it's worth noting that the 
combination of a two character keyword (i.e. if), plus a single space, 
plus an opening parenthesis creates a natural 4-space indent for the 
subsequent lines of the multiline conditional. This can produce a visual 
conflict with the indented suite of code nested inside the if-statement, 
which would also naturally be indented to 4 spaces. This PEP takes 
no explicit position on how (or whether) to further visually distinguish 
such conditional lines from the nested suite inside the if-statement. 
Acceptable options in this situation include, but are not limited to:

# No extra indentation.
if (this_is_one_thing and
    that_is_another_thing):
    do_something()

# Add a comment, which will provide some distinction in editors
# supporting syntax highlighting.
if (this_is_one_thing and
    that_is_another_thing):
    # Since both conditions are true, we can frobnicate.
    do_something()

# Add some extra indentation on the conditional continuation line.
if (this_is_one_thing
        and that_is_another_thing):
    do_something()

The stated text says that it takes no explicit stance, but proposes options to mitigate the visual conflict. Pycharm will favor the third option and flag the missing indentation level if not added.

@stefanoborini , I never stated I was not wrong with regard to pep8 which I could not care less ( as pep8 is a recommendation, not a RFC MUST or SHOULD), I care about the size of the diff produced by people in my team, so with this regard, black is playing a wonderful job, having the closing ): on a separate play for me the role of visual delimiter with the body, and help reduce diff-size when arguments are added. So the purpose of my comment was to express my support to black maintainers.

but “wrong visually” is subjective (as stated previously) , end of the story. So I don’t think your comment is bringing anything to the discussion, you’ve already showed to everybody your holy book (which is a recommendation), do you plan on replying until everybody bows to the one true way of having a not visually wrong code?

Unless you plan on writing a PEP that makes PEP-8 mandatory so that you can open a PR in cpython to reject any “visually wrong” code, I think you just have to agree to disagree with us, the internet is big enough for different opinions.

if I was to troll I would say that PEP-8 says that further indentation should be used when it’s not distinguishable with the line below, with the “):” being on its own line, I can argue that it becomes distinguishable.

especially if you look at the page on multi-line if , where PEP-8 states that they recommends no specific solution but only propose some possibility.

So with these in mind, having black that is consistent on all multi-line statement (be it if statement, function declaration, function call etc.) seems even more rational to me.

@jarrellmark I disagree. Having adjacent lines with different concerns at the same level of indentation is more confusing than Black’s approach. With Black, you know that if adjacent lines are at the same level of indentation, they are part of the same block/scope. Option (1) puts the the end of the signature and the body at the same indent level (confusing), and option (2) puts the end of the signature at the same indent as the args. I dislike this second option for two reasons. The first is that with type-hinting, I think it is better to have the function args and the return type be very easy to separate visually. The second is that I think the homogeneity with other constructs is a good thing. e.g. how we do this:

my_list = [
    1,
    2,
    3,
]

rather than this:

my_list = [
    1,
    2,
    3]

The visual consistency between the two similar concerns (how to style a lists of things bookended in some form of paren/brace) is nice to have.

This:

def add(
        first_number: int,
        second_number: int) -> int:
    return first_number + second_number

Should be (1)

def add(
        first_number: int,
        second_number: int
    ) -> int:
    return first_number + second_number

or (2):

def add(
        first_number: int,
        second_number: int
        ) -> int:
    return first_number + second_number

I prefer 1 tbh. Both are pep8 compliant.

flake8 (pylint?) has no problem with my Black-formatted code.

autopep8 reformats it, and pycharm complains about it in some cases.

1. Folding Python code that relies purely on indentation was fragile to begin with and would break on a lot of valid code (maybe not PEP8 compliant code, but such a feature shouldn’t be relying on PEP8 compliance anyway) – this is not Black’s fault.
2. Yes, this is true. I don’t see, however, why this is a concern. Unless your argument list is 50 args long (it shouldn’t be), it is always easy to see when entities at level 1 are function arguments with Black’s style. Likewise, you know that anything after a “):” is a function body, wherever that “):” appears. I have never seen anyone get confused and think that the args are actually part of the body with Black’s style. In fact, I think the nice thing about black’s style is that it provides consistency with other situations – in basically all other parts of Python code, it is convention that the closing paren/bracket/curly brace should be at the same level of indentation as the initial part of the expression, e.g. when creating a list, calling a function, etc.
3. Agreed. That is why Black indents the arguments even though it doesn’t actually need to. You could have no indentation for the args, which given the parens would be interpreted just fine by the python parser. Instead, Black chooses to indent the args once to make it clear that these are part of the function definition started above (one indentation level back). But not twice, which is gratuitous whitespace.
4. flake8 (pylint?) has no problem with my Black-formatted code. Can you provide an example of Black-formatted code that flake8 is complaining about?

I’ll add my own additional point in favor of Black’s style: 5. “Own-line Frowny Face” is better for type-hinting, as it makes function return types cleaner, as your return type is on its own line (in the middle of the face). e.g.

def add(
    first_number: int,
    second_number: int,
) -> int:
    return first_number + second_number

To my eye, this is much cleaner than the PEP-8 recommendation on this subject (which was authored before Python type-hinting existed, and I don’t think has been revisited since then):

def add(
        first_number: int,
        second_number: int) -> int:
    return first_number + second_number

@allan-simon I am sorry but you are wrong, and black is wrong. There’s no space for discussion or ambiguity. pep8 states it clearly. pylint flags it. autopep8 reformats it in the correct way. black can kick and scream and its proponent can say that the frowny face is all they need but they are wrong. They are wrong visually, and they are wrong according to pep8. End of story.

The explicit statement from PEP 8 is arguing against a different formatting choice than the one Black makes. Perhaps we could propose a clarification to the text of PEP 8 to make it clear that Black’s choice is another acceptable option.

autopep8 (which you cited a few times) is just another third-party tool, and it has no more claim to be authoritative than Black does. If Black formats things differently than autopep8, so be it; users can choose which tool fits their needs better.

Finally, as @zsol said you’re totally free to fork Black. We’ve had people fork Black in the past to enforce single quotes, change the default line length, and use tabs instead of spaces. If you choose to add yours, good luck!

@mk-pmb it should not be a mod behavior. The right behavior should be the default.

@zsol Meanwhile if you could reopen it that would be grand.

Meanwhile I found we carry an “autopep8” tag in the Github repo tags. Indeed, that’s inconsistent then.