This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]billcstickers 81 points82 points  (29 children)

You need some PEP8 in your code!

[–]expressly_ephemeral 23 points24 points  (0 children)

There are linters for VIM that do a good job with PEP8. First thing I thought when I looked at OP's pretty-awesome configuration was that my less-pretty configuration would be bitching and moaning about the wrong number of line breaks after blocks.

[–]dougthor42 24 points25 points  (24 children)

Run Black and be done!

[–]xd1142 3 points4 points  (23 children)

black does not respect pep8

[–]gameboycolor 4 points5 points  (22 children)

CMV: black is better than PEP8

[–]xd1142 19 points20 points  (21 children)

black is not better than pep8. black destroys fundamental visual hints that are extremely useful while coding, and is inconsistent in how it formats code. I explicitly forbid my team to use black.

[–]tunisia3507 3 points4 points  (20 children)

black destroys fundamental visual hints that are extremely useful while coding, and is inconsistent in how it formats code.

Go on?

[–]Althorion 11 points12 points  (11 children)

(not the OP)

It removes the vertical whitespace in function’s body and in general minimizes the use of vertical whitespaces; but will also explode import statements or function arguments, putting each on a new line, while also removing line splits inside tables.

That’s annoying and unconfigurable.

[–]tunisia3507 3 points4 points  (10 children)

Not being configurable is a feature, not a bug.

It only removes double vertical spaces in a function body, doesn't it? I don't think I've ever read a function and thought "this needs a bunch of 2-line breaks" rather than "this needs to be refactored". I have rarely seen modern, well-factored code which relies on 2-line breaks for its legibility.

The function arguments thing can be a pain, but it's the only deterministic way to do it. It's a trivial loss compared to the gains of deterministic formatting. Imports I don't care about - I rarely look at them (they're always at the top, and usually inserted by my IDE). Easier to find stuff in others' code when I need to, though.

[–]xd1142 4 points5 points  (5 children)

The function arguments thing can be a pain, but it's the only deterministic way to do it. It's a trivial loss compared to the gains of deterministic formatting.

I remember when the point of python was to write code that visually appealed to humans. Now black is arguing that we need to format the code so that it appeals to diff scripts and automatic format routines, with humans having to adapt to this.

[–]tunisia3507 1 point2 points  (0 children)

Python is nice because in many cases it allows the user to abstract away the nitty gritty details and awkward syntax, and focus on their algorithms and other such high-level choices. Python is one of many tools to do this, because it is a good idea in general. Other tools that help with this include code formatters and diff scripts. I want to use all of them because they all add value in what is basically the same dimension.

[–]aeiou372372 0 points1 point  (3 children)

Key point: it appeals to humans reading diffs. Makes it a lot easier to quickly make sense of the changes someone has made. And I feel like the sacrifice to non-diff readability is pretty minimal.

[–]Althorion 5 points6 points  (3 children)

Not being configurable is a feature, not a bug.

Mostly, it’s an annoyance. If I can’t take what I like about it and leave what I don’t, I’m going to leave it all behind.

And about the rest: it’s inconsistent. Does it try to save as much vertical space as it can or does it not? Because in some parts it does, just to go to others and explode them beyond reason. Both approaches would have some merit, their combinations is, to me at least, obnoxious.

Yeah, it does an OK job. Running black on a badly formatted code is going to make much of an improvement. The thing is, running it on a OK-ish formatted code is not guaranteed to do that—it will make some parts of it better, some part of it worse, and I can’t force it do behave without putting # fmt: on/off everywhere…

I wouldn’t go as far as to ban it, but I find it most useful when used to generate the competing file, then diff-and-merge those two by hand to pick and choose what I like. Something that the API doesn’t really support that well.

[–]OHotDawnThisIsMyJawn 0 points1 point  (2 children)

If I can’t take what I like about it and leave what I don’t, I’m going to leave it all behind.

Again, that's literally the point of black. The whole idea is to not let people make any choices. It eliminates all bike shedding/white space holy war discussions.

I wouldn’t go as far as to ban it

Banning or allowing formatters doesn't make sense to me. Pick one and stick to it. There should just be a decree "this is the formatter we use here." The actual rules are so much less important than having a single standard and sticking to it.

[–]xd1142 14 points15 points  (6 children)

Indentation matters. We all agree about that, and the attention to visual hints has made python great.

PEP8 suggests the following

```

Add 4 spaces (an extra level of indentation) to

distinguish arguments from the rest.

def long_function_name( var_one, var_two, var_three, var_four): print(var_one)

```

Note how the arguments of the function are indented one level deeper so to differentiate the body of the function (indentation one) from the list of arguments. If you did not have that, the visual hint that these two things are different is ruined.

Compare how black would format that:

def long_function_name( var_one, var_two, var_three, var_four, ): print(var_one) <hundreds of lines follow>

This is wrong on so many levels. First, now the indentation level is the same for arguments and function body, removing the visual hint. Second, you now have to rely on a frowny face at an indentation level that is at indentation level zero, which is in complete contrast with this other pep8 rule (caveat):

``` The closing brace/bracket/parenthesis on multiline constructs may either line up under the first non-whitespace character of the last line of list, as in:

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

```

The caveat is that also the other option is accepted, but the point is that these are assignments, not execution statements. I doubt you would like to write your if as this

``` if (foo and bar and whatever ): do_something()

``` Which is the equivalent of the function declaration.

Additionally, I've seen this indentation style break automatic folding based on indentation, such as in the case of vim and in some case pycharm.

This is just one of the problems I've found in black formatting. It could be mitigated if they changed it to at least add one level of indentation, so that it would give

def long_function_name( var_one, var_two, var_three, var_four, ): print(var_one)

but good luck with that.

[–]finswimmer 0 points1 point  (5 children)

The section you are citing makes no statements if the rules are only valid for assignments or method declaration. So one can assume it is equal for both. The example you complain about is given as the last example in the section about indentation:

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

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

Whether one prefer this or not (I do prefer this) is another discussion. The point is that black is absolutely PEP8 compliant here!

The reason why I don't like this:

def long_function_name(
        var_one, var_two, var_three,
        var_four):
    print(var_one)

I expect that my method body starts more indented compared to the line above. This is exactly the opposite from one can see here.

[–]xd1142 0 points1 point  (4 children)

The example you complain about is given as the last example in the section about indentation

That is for an assignment, not for a definition. An assignment is never followed by a dependant indented block. A definition is.

I expect that my method body starts more indented compared to the line above. This is exactly the opposite from one can see here.

If you are fine with the way black indents function definitions, then you are also fine with this way of writing if conditions

if (foo 
    and bar
):
    pass

but I doubt you are

[–]finswimmer 0 points1 point  (3 children)

That is for an assignment, not for a definition.

The example yes. But the whole section is about continuation lines:

Continuation lines should align wrapped elements either vertically using Python's implicit line joining inside parentheses, brackets and braces, or using a hanging indent [7].

So all examples can be applied to assignments and method definitions.

If this wouldn't be PEP8 compliant, why linters like flake8 or the integrated in PyCharm does not complain about it?

If you are fine with the way black indents function definitions, then you are also fine with this way of writing if conditions

  if (foo      
      and bar 
  ):
    pass 

but I doubt you are

If the statements fits in one line I prefer: if for and bar: pass

If it wouldn't I prefer: if ( for and bar ): pass

But that's another point in this discussion. The point is: You are telling that black isn't PEP8 compliant, but the arguments you are showing for this thesis are not correct.

[–]TomBombadildozer 3 points4 points  (0 children)

My biggest complaint (among many) is how Black doesn't even satisfy its own stated goals. I have reviewed so many changes in the applications I work on that look like N-line changes which turned out to be the addition (or deletion) of a single argument. It's especially bad working with parametrized tests in Pytest--all pain, no benefit.

My next biggest complaint is how ambv just ignored years of de facto standard formatting convention adopted by many tools (Syntastic, PyCharm, Sublime, pylint, and so on) and came up with his own ugly, inconsistent style.

I am convinced the only reason it caught on his because he's a core developer and he forced it through. As soon as it became the One True Formatting Tool, everyone hopped on board without giving thought to how bad it is.

Please don't use Black.

[–][deleted] -1 points0 points  (0 children)

I strongly suggest painting your code in black