-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
One space between class definition and the first inner method #619
Comments
What a coincidence, I used PEP8 states that:
So I guess this applies for the first method too, including |
Just tested how black would format Robot Framework source code and this was the biggest problem. I found it important to surround methods with an empty line and think it should be done consistently. I understand that the motivation behind black is remove the need to discuss about different formatting alternatives, but in this particular I think black just behaves wrong. As already pointed above, PEP-8 explicitly states that
In my opinion the PEP-8 compatible behavior should be the default. There could be a command line option to omit the empty line to avoid changes with existing blackened code bases. Having an option to preserve/add it would be the minimum. |
I think black’s behaviour make sense for classes that start with a docstring or comment. In this case, the empty line is wanted: class C:
"""Abstraction for B to do spam."""
def __init__(self):
... |
@merwok I totally agree there should be an empty line after the docstring. I think the docstring PEP also explicitly says so. This issue is about black removing the empty line if there's no docstring, even though that's against PEP-8. Without the docstring, black would format you example like this: class C:
def __init__(self):
... |
This is interesting. While your reading of PEP 8 is correct to the letter, I'm not quite sure if it was intended by the PEP 8 authors. Case in point: the PEP also says the following:
Following your logic, if a Python file starts with a function or a class definition, we would have to put two blank lines before it, no? I'd like to understand your need for this blank line better. What purpose does it serve? There's clear delineation due to the increased indentation. In fact, I'm leaning towards implementing #450 so that we are consistent in that regard. |
pep8 or flake8 used to require that and it was annoying! |
FWIW my take on the question here: class CoolRegularClass:
"""Nice docstring.
More info.
Cool.
"""
def __init__(self):
# nice to have one blank after the docstring
...
class SomeInternalClass:
def __init__(self):
# no docstring, no needless blank line
... In other words: the |
Some reasons why I prefer an empty line before the first method in a class:
I guess this is something where opinions differ and in my opinion stuff like this should be configurable. I know Black tries to be uncompromising, but there's already similar |
Reasons for blank line, IMHO:
Instead of forcing blank line or not, what if it were just left alone? I.e. allow zero or one blank line, as is currently done for the |
I'd prefer our current formatting: playground. Which is no empty line before functions, but one before other body statements. After discussing with others as well, we'll not be making any changes here. |
What is the private discord link for? |
It's a link to a public channel where we discuss development-related things. |
yeah my bad for not providing context, we are partnered with Python Discord (https://discord.gg/python) so we have our own |
It's unfortunate you don't want to make this configurable for projects like you've done at least with quotes. Being able to configure should there be empty lines or not, or at least having an option to just leave them as-is, would have been great. I like the idea of uniform formatting a lot, but I consider it a lot more important within a project than with all different Python projects. Having some things carved to stone is fine as well, but at least with behavior where PEP-8 is ambiguous configuration would make sense. Anyway, it's your project and your rules. For me this just means I won't be using Black. |
This unfortunate rule seems to be based on a flawed reading of PEP 8. PEP 8 states:
To me, this directive seems pretty unambiguous, but the maintainers have interpreted it as:
This, of course, is not the same thing. It turns out that the directive actually used to say “separated”, but in 2015 it was specifically changed to “surrounded”. See https://hg.python.org/peps/rev/5784. Thus, the current wording appears to be quite deliberate and, in all likelihood, is intended to apply to the first method definition as well. I mean, why wouldn’t it be?
This argument is disingenuous. The purpose of vertical whitespace is to separate elements of code from one another. The rules governing it are not meant to apply at either end of a file. I have read the prior discussion, and perhaps part of the problem is that the examples considered tend to be stubs and not representative of real code.
Yes, I would agree that inserting a blank line there would be silly and ugly. But as soon as the class body grows to contain methods or other blocks that are themselves separated by blank lines, the leading blank line also becomes essential. I hope that the above historical finding lends enough support to the literal interpretation of the PEP 8 directive in question that the decision to reject this issue can be reversed. Compliance with PEP 8 would seem to require that blank lines between adjoining This whole thing popped up on my radar because the rule discussed here is now being used as an argument in favor of preserving PR #3035, which has proved a controversial and disruptive change. |
Thank you for the whole of the comment - it's well written, the historical finding is interesting, the interpretation of it and the argumentation built upon it is funny to some extent. There are a lot of different code styles out there, many are inconsistent, some have written rules about them ("guidelines"), some of these are published to the general public, and possibly there are even some guidelines out there that are unambiguous. What is Black? Published on its web page:
What is PEP 8? What is PEP 7? When have these two PEPs been introduced, to apply to which projects (answer: CPython itself), for which purpose? The point I am aiming to make here: Given the historical background of PEP 8, it might not be wise to take single phrases out of it and to try to interpret it to the letter. It might not be wise for Black to refer as first thing to it, because it is generally vague and generally misunderstood by large parts of the Python community - leading to a lot of irritating discussions about nothingness. Maybe Black should put the "PEP 8 compliant" label better into their FAQ, along with explanation that PEP 8 is vague, depends on interpretation, and, importantly, PEP 8 just cannot be interpreted to the letter, given the important - and often unnoticed - starting sections "Introduction" and "A Foolish Consistency is the Hobgoblin of Little Minds". One just cannot be strict about PEP 8, there's the hobgoblin section! Let's keep the section in mind whenever we read or write or think about PEP 8.
The message that came along with the linked change set is unfortunately just "Separate -> surround." The "separate" phrasing, when read to the letter, could be imagined to apply only in cases where one method follows another method. But not in cases where there is a method, then one or more class attributes, and then another method definition. By using the wording "surrounding", when read to the letter, would then also have to be applied to method-attribute-method cases. In the presence of the hobgoblin section, it does not matter much which of these interpretations, or both, or none of them are accurate.
I think the argument wanted to provoke reflection about the interpret-PEP-8-to-the-letter approach. It resulted in applying opinionated common sense rather than literal interpretation. This is revealing, and gives a clear hint that this whole issue is just about a formatting opinion. There's nothing wrong with that, but let's rather be clear about it. |
Any hopefuls still following this issue will be delighted to learn that it has finally been fixed by PR #4130. |
Can Black have a rule to not remove the whitespace at line
2
from here:so that it will keep the blank line between class definition and any first inner entity (as it does already with the docstrings and assignment attributes)?
It's extremely ugly to have that whitespace consistent in the entire codebase and just in this particular situation, where you don't have anything to add between class definition and the first inner method (which usually is the
__init__
one), to end up with "glued" headers of class and method definition.Now if I'm formatting this:
I get this:
So I expect the rule of one empty line before and after class methods to be consistent here. Everyone likes to have symmetry in their code.
The text was updated successfully, but these errors were encountered: