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

Blank line after docstring... or not. #2370

Closed
PabloLec opened this issue Jul 12, 2021 · 17 comments
Closed

Blank line after docstring... or not. #2370

PabloLec opened this issue Jul 12, 2021 · 17 comments
Labels
F: empty lines Wasting vertical space efficiently. T: style What do we want Blackened code to look like?

Comments

@PabloLec
Copy link

PabloLec commented Jul 12, 2021

Describe the style change

Currently, black does not check blank lines between functions docstring and body.
That means both these options are valid:

def foo():
    """docstring"""
    
   return 0
def foo():
    """docstring"""
   return 0

As black is uncompromising and opinionated, I guess this should be taken into account.

Personally I tend to put a blank line but looking at the only potential reference PEP 257, there is no blank lines in given examples.

Whether we enforce the blank line or not, black should take care of this as this results in easy style inconsistency.

I'd say we should follow PEP 257 no blank line example.
Comment your opinion so that this issue can be refered to in a PR fixing this.

If there is no consensus, we could organize a jousting match to settle this. :goberserk:

@hukkin
Copy link
Contributor

hukkin commented Jul 12, 2021

Isn't this a case of (referring the docs):

Black will allow single empty lines inside functions, and single and double empty lines on module level left by the original editors, except when they’re within parenthesized expressions.

So working as intended and documented.

we could organize a jousting match to settle this

Reasonable proposal, I like this!

@PabloLec
Copy link
Author

Well, indeed the doc describes this behavior, but I think this should be treated separately.
As on module level, black does not allow beginning blank lines and enforce a final blank line, this same logic should apply at function level.

e.g. this code looks inconsistent and should be reformatted:

def foo():
    """docstring"""
   return 0

def bar():
    """docstring"""

   return 0

def baz():
    """docstring"""
   return 0

@tolomea
Copy link

tolomea commented Aug 16, 2021

I agree and would also like to see Black have an opinion in the case where there is no docstring, my personal preference is no line between function def and first line of the body, but having it take a position one way or the other matters more to me than which position.

echoing @PabloLec this code looks inconsistent and should be reformatted.

def foo():
   return 0

def bar():

   return 0

def baz():
   return 0

@stinos
Copy link

stinos commented Oct 4, 2021

Also: #1872

@stinos
Copy link

stinos commented Oct 14, 2021

Also related, perhaps not worth a separate issue (please mention if it should be): the placement of the last triple quotes for last line of the docstring itself.

Sample; all of these are left as-is, but most of these should in my opinion get a treatment. This is probably my number one issue with black. Please stick with being opinionated :)

class Foo1:
    """Multi.

    Line."""

    pass


class Foo2:
    """Multi.

    Line.
    """

    pass


def Func1():
    """Single."""
    pass


def Func2():
    """Single."""

    pass


def Func3():
    """Multi.

    Line."""
    pass


def Func4():
    """Multi.

    Line."""

    pass


def Func5():
    """Multi.

    Line.
    """

    pass


def Func6():
    """Multi.

    Line.
    """
    pass

On one hand doing the same as for single-line docstrings, triple quotes on same line and no newline after, it is consistent.

On the other hand the example for https://www.python.org/dev/peps/pep-0257 shows the triple quotes on the next line and no blank line after, which seems to be used most in the standard library (only had a quick look though).

Since this is about opinions, let's express one: I'd be in favor of consistency so no blank line after (at least for functions, for classes and modules should remain as-is) and triple quotes attached to the text (after all that's how the docstring starts as well).

@felix-hilden felix-hilden added the T: style What do we want Blackened code to look like? label Oct 14, 2021
@felix-hilden
Copy link
Collaborator

felix-hilden commented Oct 14, 2021

I'm not 100 % sure if we should do it. I'd say I'm moderately in favor of it 😄 But if we do, my preference would be no blank lines:

def f():
    pass

def f():
    """Doc."""
    return stuff

def f():
    """
    Multiline doc.

    With some explanation.
    """
    return stuff

That might be influenced a lot by the fact that an IDE will highlight the docstring very differently from the following code, which is a good visual aid. But most people should be using one anyway.

@felix-hilden felix-hilden added the F: empty lines Wasting vertical space efficiently. label Oct 14, 2021
@stinos
Copy link

stinos commented Oct 14, 2021

def f():
    """
    Multiline doc.

    With some explanation.
    """
    return stuff

Now that you mention it; I didn't consider the start of doc strings, but that's another case that should imo be treated and the newline erradicated; I'd like

def f():
    """Multiline doc.

    With some explanation."""
    return stuff

or perhaps the seemingly more common

def f():
    """Multiline doc.

    With some explanation.
    """
    return stuff

but not both, and also none of the other shenanigans like extra newlines before the opening """ or after the opening """. We have files like that lying around with some of these mixed, and it looks really off. And one uses black to fix that, normally. I get that docstrings might allow for some creativity, but at least fixing the beginning and end seems worth it.

@felix-hilden
Copy link
Collaborator

Well that's a can of worms too 😅 but I definitely prefer either on one line, or all at the same level, like I wrote above.

@stinos
Copy link

stinos commented Oct 14, 2021

I looked around a bit and this at the end of strings.fix_docstring (adjust to taste :))

    # Always attach text to the opening triple quotes.
    while len(trimmed) > 1 and not trimmed[0].strip():
        del trimmed[0]
    # Need to get rid of the indentation again if empty lines were deleted.
    trimmed[0] = trimmed[0].lstrip()
    # Always place trailing triple quotes on separate line if multi-line.
    if len(trimmed) > 1 and trimmed[-1].strip():
        trimmed.append(prefix)
    # Get rid of empty lines before trailing triple quotes.
    while len(trimmed) > 1 and not trimmed[-2]:
        del trimmed[-2]

seems to fix first/last lines of docstrings, combined with

        if (
            self.previous_line
            and current_line.is_triple_quoted_string
        ):
            return 0, 0

at the end of lines.EmptyLineTracker._maybe_empty_lines to get rid of lines before/after. That last one is almost certainly not sufiicient for things like docstrings in the middle of a class, but just to give an idea.

@felix-hilden
Copy link
Collaborator

@tolomea Your suggestion (without the docstrings) is being tracked in #902.

@PabloLec
Copy link
Author

PabloLec commented May 20, 2022

Just a quick bump, I tried reaching out and offer help on main communication means a few times within the past months but no response so, yeah.
I guess there are more important matters and blocking issues to consider first but this one seems close to black main purpose to me.
Again, whatever the formatting will be, we need one.

@felix-hilden
Copy link
Collaborator

Module docstrings are already being handled by #2996. It's not a big stretch to start applying that elsewhere as well. We'll just have to agree to do it. And us (potentially) removing the lines in module docstrings would be in line with my preference above.

@mcnoat
Copy link

mcnoat commented Mar 28, 2023

I vote for no blank line after the docstring. I discovered this issue, because I was rewriting a file so that it complied with pydocstyle's standards. No matter which docstring style you use (PEP 257, numpydoc or Google), according to pydocstyle you should never have a blank line after a docstring. (source: http://www.pydocstyle.org/en/stable/error_codes.html: "No blank lines [...] after function docstring" is error code D202 and it's included in all three style conventions).
I had to manually delete the trailing line for each function and would have loved it if black had taken care of it already.

@YodaEmbedding
Copy link

I know some tools will enforce PEP 257, but for non-tiny functions, it just looks weird to have no blank line, e.g.:

@torch.no_grad()
def inference(
    model: CompressionModel,
    x: torch.Tensor,
    skip_compress: bool = False,
    skip_decompress: bool = False,
    criterion: Optional[TCriterion] = None,
    min_div: int = 64,
    *,
    lmbda_idx: int = None,
    lmbda: float = None,
) -> dict[str, Any]:
    """Run compression model on image batch."""
    if lmbda_idx is not None:
        assert lmbda is None
        lmbda = model.lambdas[lmbda_idx]

    ...

versus:

@torch.no_grad()
def inference(
    model: CompressionModel,
    x: torch.Tensor,
    skip_compress: bool = False,
    skip_decompress: bool = False,
    criterion: Optional[TCriterion] = None,
    min_div: int = 64,
    *,
    lmbda_idx: int = None,
    lmbda: float = None,
) -> dict[str, Any]:
    """Run compression model on image batch."""

    if lmbda_idx is not None:
        assert lmbda is None
        lmbda = model.lambdas[lmbda_idx]

    ...

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 13, 2023

As mentioned in #4043, I lean towards not doing anything here.

I feel that if we do something here, it should not be to always strip blank lines. Doing so would run in to the same readability issues that has made previous blank line changes controversial (see the comment above, #4043, etc). It would be inconsistent with how Black treats module and class docstrings. The examples in this issue arguing for stripping the blank line are all toy code.

@LLyaudet
Copy link
Contributor

LLyaudet commented May 3, 2024

Since Black has the goal of not allowing too much customization to avoid too many variations out in the nature,
I think the original suggestion to not allow an empty line after function doc string is better:

  • it corresponds to PEP257 (DocString)
  • I would allow doc strings with either
    """BlaBla"""
    or
    """
    Blabla
    Blabla
    """
    In both cases, the separation with code below is clear.
    Blank line after docstring... or not. #2370 (comment)
    seems invalid to me.
    Because if you have a big function and a small docstring on only one line,
    the problem is in your too short docstring.
    I would imagine a big function would have some big parts,
    each part being synthetized in the docstring like:
"""
My function:
- part 1: blabla
- part 2: blabla
- part 3: blabla
"""

And inside function body:

# Part 1 : Blabla
# Blabla
# Blabla
bla = blabla(x,y,z)
...
# Part 2 : Blabla
# Blabla
# Blabla
bla = blabla(x,y,z)
...

etc.

Moreover in linked issue #902, someone argued about the case of a comment just after the docstring:
#902 (comment)
I think his example is bad, since if I remember correctly in PEP8, the comment should be inside the if block and not outside.
But still, there may be a case where "part 1" starts with a function call just after docstring.
And that case with a corresponding example would be relevant to the comment of @jenstroeger.
I would not change my opinion for that and either accept that:

"""
My function:
- part 1: blabla
- part 2: blabla
- part 3: blabla
"""
# Part 1 : Blabla
# Blabla
# Blabla
bla = blabla(x,y,z)

is slightly ugly.
Or that:

"""
My function:
- part 1: blabla
- part 2: blabla
- part 3: blabla
"""
#
# Part 1 : Blabla
# Blabla
# Blabla
bla = blabla(x,y,z)

is also slightly ugly and maybe more readable,
or

"""
My function:
- part 1: blabla
- part 2: blabla
- part 3: blabla
"""
# ----------------------
# Part 1 : Blabla
# Blabla
# Blabla
# ----------------------
bla = blabla(x,y,z)

is ok.
I think the trick of the empty line switched to an empty comment line,
or a comment line with dashes is enough to have a nice visual and stop brainfucking about corner cases.

@JelleZijlstra
Copy link
Collaborator

I agree with @hauntsaninja above; we'll leave this unchanged.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: empty lines Wasting vertical space efficiently. T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

11 participants