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

Please respect the established Python coding style, as exemplified by the standard library #1252

Closed
grothesque opened this issue Jan 30, 2020 · 9 comments
Labels
T: style What do we want Blackened code to look like?

Comments

@grothesque
Copy link

grothesque commented Jan 30, 2020

The idea behind Black is nice, but please consider changing it in such a way that it respects the established body of code styling best practice as exemplified by PEP8 and the Python standard library. (In particular, Black should respect the 80 character line width limit that is not only mandated by PEP8 but has been a best practice in programming for decades, and that for good reasons.)

Take, for example, asyncio, a recent project that Guido van Rossum has been personally involved in and that features a consistent and pleasant coding style. By all means, asyncio is an exemplary Python library. Black should not have to restyle it in striking ways, while currently it does.

Consider, for example, the following nicely formatted code taken from asyncio/base_events.py:

class Server(events.AbstractServer):

    def __init__(self, loop, sockets, protocol_factory, ssl_context, backlog,
                 ssl_handshake_timeout):
        self._loop = loop
        self._sockets = sockets
        self._active_count = 0
        self._waiters = []
        self._protocol_factory = protocol_factory
        self._backlog = backlog
        self._ssl_context = ssl_context
        self._ssl_handshake_timeout = ssl_handshake_timeout
        self._serving = False
        self._serving_forever_fut = None

# ...

    def _start_serving(self):
        if self._serving:
            return
        self._serving = True
        for sock in self._sockets:
            sock.listen(self._backlog)
            self._loop._start_serving(
                self._protocol_factory, sock, self._ssl_context,
                self, self._backlog, self._ssl_handshake_timeout)

Black turns it into something that, while it respects the letter of PEP8, is against established practice:

class Server(events.AbstractServer):
    def __init__(
        self,
        loop,
        sockets,
        protocol_factory,
        ssl_context,
        backlog,
        ssl_handshake_timeout,
    ):
        self._loop = loop
        self._sockets = sockets
        self._active_count = 0
        self._waiters = []
        self._protocol_factory = protocol_factory
        self._backlog = backlog
        self._ssl_context = ssl_context
        self._ssl_handshake_timeout = ssl_handshake_timeout
        self._serving = False
        self._serving_forever_fut = None

    # ...

    def _start_serving(self):
        if self._serving:
            return
        self._serving = True
        for sock in self._sockets:
            sock.listen(self._backlog)
            self._loop._start_serving(
                self._protocol_factory,
                sock,
                self._ssl_context,
                self,
                self._backlog,
                self._ssl_handshake_timeout,
            )

Please consider to not reflow (argument) lists, except perhaps where this is strictly necessary to avoid too long lines. After all, Black is already fine with not normalizing away blank lines in code, so it could just as well also respect the programmer's choice of list formatting, which often carries meaning, like in

def f(x, y, z,
      sparse=False,
      some_long_option=None,
      another_long_option=None):
    pass

that Black turns into

def f(x, y, z, sparse=False, some_long_option=None, another_long_option=None):
    pass

Note that if yet another option is added, Black completely changes the style to:

def f(
    x,
    y,
    z,
    sparse=False,
    some_long_option=None,
    another_long_option=None,
    yet_another_option=None,
):
    pass

Where is the visual consistency here? Besides, the above must be hurting not only my eyes, or this style would have found adoption in the Python standard library.

@grothesque grothesque added the T: style What do we want Blackened code to look like? label Jan 30, 2020
@zsol
Copy link
Collaborator

zsol commented Jan 30, 2020

Hey @grothesque thanks for the thoughtfully worded issue, I really do appreciate it.

You raise two high level points:

  1. the default line length: Consider switching line-length default to 79 chars, per PEP8 #498 talks about changing the default line length to a value used by the standard library. We still believe the current default is better for the reasons outlined in the readme.
  2. Respecting the user's formatting choice for argument lists. One of the guiding principles of Black is that the input formatting shouldn't influence the output (Black does violate this principle, but only rarely). The whole point of a formatter like Black is for the human to give up control of code formatting so they can instead focus on the content.

The reasons for the style choice in your last example are also explained in the readme: if it fits on one line, it goes on one line; otherwise Black splits on all arguments.

@zsol zsol closed this as completed Jan 30, 2020
@grothesque
Copy link
Author

grothesque commented Jan 30, 2020

Hello @zsol,

It took me quite some time to organize my concerns and write them down above. I did this with the best intentions for this promising project. Closing my issue after only a few hours takes others interested persons the chance to comment.

  1. the default line length: Consider switching line-length default to 79 chars, per PEP8 #498 talks about changing the default line length to a value used by the standard library. We still believe the current default is better for the reasons outlined in the readme.

The only reason given in the readme file is that Raymond Hettinger thinks that it’s a good choice. Other people disagree: most programming styles (including PEP8) mandate 80 characters or less.

80-character-lines are the standard in programming since the time of punch cards. Such standards are useful. Limiting code to 80 characters means it can nicely fit in email, on websites, in side-to-side diffs. People can keep text editor and terminal windows 80 characters wide (and fit two or three of them next to each other), etc.

As noted in the Linux kernel coding style, when people believe to have too little horizontal space for their code, this is usually a clear sign that they should reorganize their program.

Finally, 80 (or 79, if you want to be really strict) is the value mandated by PEP8. As a tool developed under the PSF umbrella, Black should respect PEP8 or PEP8 should be modified.

  1. Respecting the user's formatting choice for argument lists. One of the guiding principles of Black is that the input formatting shouldn't influence the output (Black does violate this principle, but only rarely). The whole point of a formatter like Black is for the human to give up control of code formatting so they can instead focus on the content.

Black’s stated principle of being “uncompromising” is absurd when applied with ultimate consequence. Proof: Black itself does not respect it, or it would have to normalize away empty lines inside functions. Shouldn't it also strip comments? So the question is not whether to be uncompromising or not, but where to draw the line.

PEP8 and the established practice (as exemplified by the standard library) provide a time-tested answer to this question. They grant us some freedoms of expression, while being strict in general. (In cases not covered by PEP8, like 0xff vs 0XFF, Black's current choices seem reasonable.) These freedoms are useful non-textual ways to comment code. A few come to mind:

  • Empty lines inside functions: that’s already respected by Black.

  • Line breaks in (argument) lists: see the first comment of this issue.

  • Hugging of operators, e.g. a*b + c*d. As suggested by @njsmith, Black could respect user-chosen hugging if it is consistent with operator precedence. That would rule out misuse like a*b+c*d and would allow people to write factor_a * factor_b.

Just imagine how much more useful Black would become if it was strict, but not little-minded. It could be adopted virtually by everybody who writes Python code without hesitation!

@grothesque
Copy link
Author

I would like to invite the project initiator, @ambv, to position himself with regard to the points that I raise, or to provide a pointer if this has been already discussed before. I believe that such clarification would be greatly appreciated by many people, since it touches the very essence of this project.

@ambv
Copy link
Collaborator

ambv commented Jan 31, 2020

@grothesque, you are two years too late to voice concerns about changing default values or other core tenets of the library. There is way over 20 millions of lines of code currently formatted with Black, a majority of it with the provided default value. It would be a major breach of trust to change that default now because a random user felt it was unpure.

Let's go through your concerns in order of appearance.

Respecting PEP 8

Black is consistent with PEP 8, both in letter and spirit. It allows you to choose the line length if you need it to be 79 characters per line or 120. Use this capability. The default will not change because thousands of teams depend on it and are happy with it.

PEP 8 says that consistency with your project is more important than consistency with the style guide.

Respecting the standard library

The standard library is 30 years old in some places. It does not represent a consistent formatting style itself. Compare configparser against dataclasses, email against typing, and so on.

Consider different reflow rules

This is an opinionated tool. If the tool does not meet your requirements, there are others that allow for very flexible configuration. The appeal of Black and the reason it made sense to start a new formatter project in 2018 is that it does not, in fact, allow flexible configuration. It is opinionated so you don't have to be.

Again, changing the rules today would be massively disruptive to all teams which already adopted Black. It's not happening.

Entitlement

I hope this is not deliberate on your end but you should know that the language of your comments reads unpleasantly insolent.

You dismissed a core developer's decision to close the issue because you expected no less than to have the original creator of the project bow to your opinion. With all due respect, man, you've shown no skin in this game. Yet you go ahead on this tracker calling the project's decisions little-minded or absurd. Don't be like that.

@grothesque
Copy link
Author

grothesque commented Jan 31, 2020

Thanks for the quick reply!

@grothesque, you are two years too late to voice concerns about changing default values or other core tenets of the library. There is way over 20 millions of lines of code currently formatted with Black, (...)

It’s true that I’m late, but I only became aware of Black when some people started submitting code with many overlong lines. Still, even now, the amount of code formatted by Black is dwarfed by the complete body of Python code ever written, that mostly tries to follow PEP8.

My point is that Black is a nice idea, but some small changes would make it much more useful. Here is my definition of “much more useful”: one could apply Black to the stdlib, or to example code from the official Python documentation, without striking changes. You are free to dismiss this as the “opinion of a random user”, but now at least that’s written down.

If you are concerned with backwards compatibility, the suggested improvements could be enabled by a command line option. You wouldn’t surprise any users, but you would gain many more, for example in the scientific programming community (see #148). PEP8 could be amended to say: if unsure, simply pipe the code through Black with the --pep8 option.

Black is consistent with PEP 8, both in letter and spirit. It allows you to choose the line length if you need it to be 79 characters per line or 120. Use this capability. The default will not change because thousands of teams depend on it and are happy with it.

PEP8 says “limit all lines to a maximum of 79 characters” and goes on to explain that individual teams may decide to raise that limit for themselves if they want, but the Python standard library won’t accept such code.

Therefore, Black by default violates PEP8 except for teams that have decided to use 88 character wide lines for themselves.

The standard library is 30 years old in some places. It does not represent a consistent formatting style itself. Compare configparser against dataclasses, email against typing, and so on.

Certainly, coding style evolves with time. Still, the style of the stdlib has been remarkably stable with respect to the points that I raise (line length, breaking of lists, operator hugging). For example, the modern typing module that you mention has perfectly readable code like

    def __call__(self, *args, **kwargs):
        if not self._inst:
            raise TypeError(f"Type {self._name} cannot be instantiated; "
                            f"use {self._name.lower()}() instead")

or

    __slots__ = ('__forward_arg__', '__forward_code__',
                 '__forward_evaluated__', '__forward_value__',
                 '__forward_is_argument__')

that Black insists on changing. You are of course free to disregard these almost 30 years of tradition. I only suggest that your project would be more useful if it respected it.

This is an opinionated tool. If the tool does not meet your requirements, there are others that allow for very flexible configuration. The appeal of Black and the reason it made sense to start a new formatter project in 2018 is that it does not, in fact, allow flexible configuration. It is opinionated so you don't have to be.

Black is not just any random formatter. Putting it under the umbrella of the Python Software Foundation anoints it as at least quasi-official. Users of Black can reasonably expect that code that they format with Black respects established standards and could, for example, be included as-is in the Python standard library.

Please note that my suggestions do not add any configuration. For backwards-compatibility there could be a switch that preserves the old behavior, but that’s all.

Again, changing the rules today would be massively disruptive to all teams which already adopted Black. It's not happening.

You mean you wouldn’t even accept a PR that implements a command line option that enables the three said changes?

I hope this is not deliberate on your end but you should know that the language of your comments reads unpleasantly insolent.

Please compare your language to mine. You are accusing me, "the random user", of being dismissive, insulting, and dominant, while I tried to be helpful, if frank.

You dismissed a core developer's decision to close the issue because you expected no less than to have the original creator of the project bow to your opinion. With all due respect, man, you've shown no skin in this game. Yet you go ahead on this tracker calling the project's decisions little-minded or absurd. Don't be like that.

Honestly, all I hoped for was a serious address of the three points that I raise (line length, breaking of lists, operator hugging), because I could not find a convincing discussion elsewhere.

Please note that I did not write that Black’s design decisions are absurd. I wrote that applying Black’s principles with with ultimate consequence would be absurd and immediately gave an example that Black does not do that. Black is not as uncompromising as it pretends to be, and my point is that it would do it good to embrace flexibility a little bit more than it does already now.

With regards to “little-minded”, this is a well-known phrase from the beginning of PEP8, coined probably by Guido himself, and it was clearly marked as such. In my opinion it underlines the issues that I’m raising very well.

@ambv
Copy link
Collaborator

ambv commented Jan 31, 2020

Please compare your language to mine. You are accusing me, "the random user", of being dismissive, insulting, and dominant, while I tried to be helpful, if frank.

There's a well-known meme affectionately called a Karen. Karen only wants to help, by which she means ordering people around. Karen is offended when people don't listen. Karen doesn't listen though. Karen wants to speak to the manager. This is not how open source should work.

First be a user. Then, come and try to help. Contribute, and while doing so, get to understand what the project is about, why it is as it is today. Finally, only after all that, you are ready to influence change. Try to force it without establishing some trust and brace for disappointment.

Why this is a bad idea

In the interest of the silent reader who found this issue and maybe somewhat agrees with Christoph, let me explain once more why we won't be doing what he'd like.

Black, the project, was started in 2018 when both YAPF and autopep8 were already mature tools. Its goal was to stop bike shedding, first within Facebook and a few projects around my immediate surroundings. To that end, the tool had no configuration besides line length (I wasn't willing to die on that hill). The initial alphas indeed also removed blank lines within functions.

To this day, there is no additional configuration of behavior. There are two toggles which allow the user to disable two particularly divisive elements of the Black style:

  • standardizing on double quotes and lowercase string prefixes; and
  • standardizing numeric representation.

You can disable those two things but there are no toggles that enable alternative formattings. This would go against the core tenet of the tool which is to solve bike shedding. If there's configuration, bike shedding only moves to the configuration file. Not having this problem is why people started using Black in the first place.

Now, Christoph demands that Black adds a formatting style toggle. In fact, he'd like for the current style only to survive as a piece of legacy needed for backwards compatibility. But there's a peculiar double standard here: Christoph rejects using the existing --line-length toggle because it defaults to a number he doesn't like. It seems he understands that configuration is not the solution here.

Finally, once again:

  • PEP 8 is a style guide for the standard library in the main Python distribution. Its use of 79 characters per line is a convention that can vary by project. Black users can configure this with --line-length=.
  • The algorithm to reflow lines which are too long by taking a pair of brackets and treating them like collection literal brackets is the core innovation of Black and is not configurable. There is one subtle feature which still needs work which is: if you put a trailing comma at the end of your bracket, Black will explode the contents per line. This is how you can prepare for growing collection literals or function calls. This doesn't always work great which is why Black is not deemed stable yet, this is an area I'll be focusing on in February.
  • Math operator hugging: I am not happy with the current behavior either but I've seen no concrete proposal to improve it so far which isn't full of its own edge cases. So far we're using what's simple. I'm open to improvements in this area, I understand this would be helpful to scientific users. This is another area I'll be looking at in February.
  • There will be no additional configuration toggles for any of this.

@grothesque
Copy link
Author

First be a user. Then, come and try to help. Contribute, and while doing so, get to understand what the project is about, why it is as it is today. Finally, only after all that, you are ready to influence change. Try to force it without establishing some trust and brace for disappointment.

Anybody interested is welcome to comment on my projects and if they take the effort to bring forward arguments, I will argue with them in the spirit of trying to find the best solution.

You are the release manager of Python 3.8. When I proposed the syntax and semantics of what became the flagship feature of Python 3.8, the situation was very similar: I came very late to the discussion (having only learned about it through an article in Linux weekly news), after two iterations of PEPs and scores of emails on python-ideas. I am not involved in CPython development, and, unlike here, I suggested a complete departure from the original design. Still my suggestion was received kindly. I’m not saying that I’m always right, far from it, but this is the kind of discussion culture that helps to advance free software.

@tupui
Copy link

tupui commented Jul 1, 2021

  • Math operator hugging: I am not happy with the current behavior either but I've seen no concrete proposal to improve it so far which isn't full of its own edge cases. So far we're using what's simple. I'm open to improvements in this area, I understand this would be helpful to scientific users. This is another area I'll be looking at in February.

@ambv are you still planning to work on math operators? I am afraid that in it's current state, we will not be able to use Black in SciPy (I cc you in the PR) 🥲 Maths operations is a particularly sensible topic for the scientific community and without changes (while still respecting the PEP8 as multiple people already showed), Black will just not be accepted.

Basically we all (the scientific community) want to be allowed to do this: (2*a + 3*b)**2 or even dfdx = sign*(-2*x + 2*y + 2)

If this is not wished anymore, I know you don't want to allow any configuration, but maybe having a plugin system would be an option?

@mrpackethead
Copy link

The above dialogue just reinforced to me why i dont' want to use black. Its focus on formating makes code un-understanable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

5 participants