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

Black should have an opinion about doc strings #144

Closed
Lukas0907 opened this issue Apr 18, 2018 · 70 comments
Closed

Black should have an opinion about doc strings #144

Lukas0907 opened this issue Apr 18, 2018 · 70 comments
Labels
T: enhancement New feature or request T: style What do we want Blackened code to look like?

Comments

@Lukas0907
Copy link

Lukas0907 commented Apr 18, 2018

Operating system: Ubuntu 16.04
Python version: 3.6.1
Black version: master
Does also happen on master: yes

Hi,

currently Black doesn't seem to have an opinion about doc strings or rather where the quotes should go.

Black claims for this file for example that it is already formatted:

def test():
    """This is one possibility.

    Very important stuff here.
    """


def test2():
    """This is another one possibility.

    Very important stuff here."""


def test3():
    """
    This is another one possibility.

    Very important stuff here.
    """


def test4():
    """
    What about this?
    """


def test5():
    """What about this?

    Some people also like to have an empty line at the end of the doc string.

    """

The tool pydocstyle (in the spirit of PEP-0257) at least complains that the closing quotes should go on a separate line and if the docstring fits one line it should only span one line.

It would be nice if that could be incorporated in Black as well.

Thanks for the great work!

Lukas.

@rinslow
Copy link

rinslow commented Apr 18, 2018

I will suggest my opinion:

"""One-liner.

Continuation for the one-liner. (Optional)

Title:
|4 SPACES|one line. might be really really really really really really really really
|8 SPACES| really long
|4 SPACES| next line (something): something something

Note:
|4 SPACES| this is just my opinion but obviously it's the best one
"""

And also:
"""One-liner documentation is closed on the same-line."""

@ambv
Copy link
Collaborator

ambv commented Apr 18, 2018

So I came here to complain that docstrings are out of scope but reading your comment actually makes me think what you're asking for is reasonable 😄

Specifically:

  • if it fits one line, it should;
  • if it doesn't, the closing quotes should be on their own line.

I'll think about possible negative effects some more before deciding whether to do this for sure but it looks like some opinions here might be harmless.

@carljm, what do you think?

@carljm
Copy link
Collaborator

carljm commented Apr 18, 2018

I think it's reasonable for Black to be opinionated about this, and I think the two rules you named are the right ones to enforce. I would extend the second rule to also have an opinion about opening quotes; I slightly prefer both opening and closing quotes on their own line for a multi-line docstring, but wouldn't object to either variant.

I'm not sure why, but I feel like consistent docstring formatting actually has a huge impact on whether code feels consistently formatted. I guess because they tend to be visibly syntax-highlighted large blocks of text.

@ambv ambv added T: enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers T: style What do we want Blackened code to look like? labels Apr 19, 2018
@natecox
Copy link

natecox commented Apr 24, 2018

I tend to favor single line where applicable:

def somefunc():
    """This is my docstring"""

Multi-line I'd personally prefer the first line be on a new line (i.e., test3 above) but I wouldn't complain about the test format above either.

@skapil
Copy link
Contributor

skapil commented May 8, 2018

@ambv Not sure, if anyone is looking into it. Can I give a try to this?

@ambv
Copy link
Collaborator

ambv commented May 8, 2018

Of course, go for it!

@willingc
Copy link
Collaborator

willingc commented May 8, 2018

Agree with the approach @ambv and @carljm put forth. Like Carl, I like the longer docstrings to have """ on their own lines. I often put a blank line before the closing """ as it adds a bit of whitespace but it's totally fine not to have the blank line.

@coxley
Copy link
Contributor

coxley commented May 9, 2018

+1 to prefer single line if possible, otherwise both opening and closing quotes on their own lines. :)

@sersorrel
Copy link
Contributor

Just to throw it out there, I personally prefer PEP 257 style, and I think that's the right thing to do if Black is aiming to follow PEP 8:

"""if it all fits on one line"""

"""if not: some title

more text
"""

but I've seen a wide variety of other styles in the wild, including these two which I don't think have been mentioned:

""" spaces either side """

"""two line one-liner
"""

I do kind of like having a blank line before the closing quotes, as it makes wrapping the body of the docstring much easier in Vim, though it's probably better to write a Vim plugin that fixes this in Vim rather than influencing Black's style.

@carljm
Copy link
Collaborator

carljm commented May 9, 2018

@anowlcalledjosh PEP 257 is explicitly agnostic about the placement of opening quotes in a multi-line docstring. To quote: "The summary line may be on the same line as the opening quotes or on the next line."

So Black can choose either of these and be equally PEP 257 compliant:

"""
Summary line.

More lines.
"""

or

"""Summary line.

More lines.
"""

My (weak) preference for the former is based on allowing three more characters in the summary line before hitting a line-length limit.

Interestingly, on the question of blank line before closing quotes, PEP257 requires it on class docstrings but not function docstrings ("Insert a blank line after all docstrings (one-line or multi-line) that document a class") because the docstring should be spaced from the first method. Black also adds a blank line (outside the docstring) before the initial method, though, which PEP257 probably doesn't anticipate. (EDIT on second read, it's not entirely clear if the PEP means for this blank line to be inside or outside the quotes; if outside, then that matches what Black already will do.)

@ambv
Copy link
Collaborator

ambv commented May 9, 2018

it's not entirely clear if the PEP means for this blank line to be inside or outside the quotes; if outside, then that matches what Black already will do.

From my read I thought I'm doing what the PEP is suggesting.

@natecox
Copy link

natecox commented May 9, 2018

I'm moderately certain PEP is telling us to add a blank line after the closing quotes

@lithammer
Copy link

Worth thinking about as well is that the docstring format will have impact on __doc__. So for example this docstring:

"""
Summary line.

More lines.
"""

Will produce this __doc__:

'\n    Summary line\n\n    More lines.\n    '

While if you start the summary line immediately you don't get the \n at the start of the __doc__ attribute.

@ambv
Copy link
Collaborator

ambv commented May 9, 2018

@Renstrom, that used to bother me in the past when help() used to write out the contents of docstrings without calling lstrip() on it first. Now it's less of an issue and tools like PyCharm already correctly strip left whitespace, too.

@brianv0
Copy link

brianv0 commented May 25, 2018

Support was recently merged into pycodestyle to support a doc string line length which differs from that of the code line length in PyCQA/pycodestyle#674. It'd be nice to have some tooling and an opinion around that.

The primary use case is to comply with a 72 character docstring limit in pep8, but it's also immensely useful if you allow line lengths of 120 but just want to make sure your docstrings are below 80 so it plays nicely with pydoc.

@sginn
Copy link

sginn commented May 25, 2018

In response to #144 (comment), The blank line should be removed if it's a docstring for a function, but retained for a class.

After running black on our codebase, Flake8 is complaining about D202 No blank lines allowed after function docstring.

def req_required(method=None):
    """Configure reqparse for the request."""

    def decorator(f):
        pass

Black is inserting that blank line between the docstring first line, but shouldn't, because req_required is not a class.

from PEP-257

Insert a blank line after all docstrings (one-line or multi-line) that document a class -- generally speaking, the class's methods are separated from each other by a single blank line, and the docstring needs to be offset from the first method by a blank line.
     ....
     Notes:
        ...
        - There's no blank line either before or after the docstring.```

@ambv
Copy link
Collaborator

ambv commented May 25, 2018

Black differentiates inner functions from the rest of the body. If you had other instructions there. It wouldn't put empty lines.

@dirn
Copy link

dirn commented May 29, 2018

This could be tricky when writing a decorator.

def decorator(f):
    """A decorator."""
    @wraps(f)
    def inner():
        return f()
    return inner

Black will add a line after the docstring, causing pydocstyle to report a D202. It sounds like the way to prevent this is to put something else between the two defs, but there isn't always anything to put there as the outer function often just returns the inner one. (I've tried both with and without wraps and the result is the same.)

@auscompgeek
Copy link

For those waiting on this, docformatter might interest you: https://github.com/myint/docformatter

@TiemenSch
Copy link

TiemenSch commented Sep 12, 2018

Regardless of which layout you choose, I think too many debatable cases will arise if Black would try to enforce a line-length in your docstrings' bodies, for example. Some examples concerning the body text:

  • If you would like to be sticking to 80, but your one-liner is too long. Should it change something? Black isn't a linter, but there is no clear solution either.
  • Line length in subsequent blocks (Arguments and what not). It is more common to continue lines there with an extra indent. But not every continuation needs to indent further and further. Say a sentence spanning three lines, then the second line needs to get an (additional) indent, but the third shouldn't indent any further. It's hard to detect when a sentence/remark ends (unless you enforce a "." at the end of every sentence)
  • Trailing whitespace? --> I feel this can be trimmed without doing to much harm?

Gets messy real quick :)

Perhaps there are too many docstring styles to enforce/output something more useful than the input in the bodies. Who knows whether someone is using Epydoc, Sphinx, Google or any other style.

So I think it's best for the "opinion" of Black to stick to the bare layout, like it is above (with an option to turn doc formatting off). And trailing whitespace could be trimmed without causing trouble AFAIK.

My preference would be one-liners and test() for the multi-line one.

@bancek
Copy link
Contributor

bancek commented Jan 9, 2019

I've implemented docstring indent fixing in my fork.

bancek@0889797

@louwers
Copy link

louwers commented Jun 10, 2020

OK, next topic. Is should Black have an opinion on parameter documentation?

E.g.

def func(param1, param2):
    """
    This is a reST style.
    
    :param param1: this is a first param
    :param param2: this is a second param
    :returns: this is a description of what is returned
    :raises keyError: raises an exception
    """
    pass

More styles here: https://stackoverflow.com/a/24385103

@ssbarnea
Copy link
Contributor

I personally found reST much harder to deal with than MD and with the introduction of type hints the need to follow its param docs seems to create undesired repetition. Sphinx itself seems to now have support for https://www.sphinx-doc.org/en/master/usage/markdown.html but I am not sure if it can be used as a replacement (i plan to test that).

What I wanted to point with that is that the future of documentation format is far from clearn. Still, this does not mean we should not work towards addressing it. There are lots of steps that would apply regardless which formats will gain more traction.

@dougthor42
Copy link
Contributor

I would say that core black should not have an opinion on docstring style (reST, NumpyDoc, Google, etc.).

If anything, docstring style adjustments could be an optional plugin/add-on or even an entirely separate package. black-docstrings or something like that.

Note that there's already blacken-docs which will format python code blocks within docstrings.

@dimaqq
Copy link

dimaqq commented Jun 11, 2020

Using both rst and md with Sphinx (recommonmark) and without Sphinx, I'll second this:

black should not touch any markup in the docstring.

@mgoldey
Copy link

mgoldey commented Jun 15, 2020

Note that there are situations where an opinion inside black would be helpful which AFAIK are not discussed above. For instance, when transitioning from 2-space to 4-space code, black indents the first part of a docstring, but not any subsequent lines, so jagged code as follows is possible:

Given

def some_functions(args):
  """ this is a test
  of some long docstrings
  which go into multiple lines
  """
  pass

black then produces

def some_functions(args):
    """ this is a test
  of some long docstrings
  which go into multiple lines
  """
    pass

This might be a separate concern.

@ichard26
Copy link
Collaborator

That has been fixed on master already:

(black) richard-26@ubuntu-laptop:~/programming/black$ black test.py --diff --color
--- test.py	2020-06-15 21:12:39.234785 +0000
+++ test.py	2020-06-15 21:12:48.966120 +0000
@@ -1,6 +1,6 @@
 def some_functions(args):
-  """ this is a test
-  of some long docstrings
-  which go into multiple lines
-  """
-  pass
+    """this is a test
+    of some long docstrings
+    which go into multiple lines
+    """
+    pass
would reformat test.py
All done! ✨ 🍰 ✨
1 file would be reformatted.

@kuwv
Copy link

kuwv commented Sep 16, 2020

OK, next topic. Is should Black have an opinion on parameter documentation?

E.g.

def func(param1, param2):
    """
    This is a reST style.
    
    :param param1: this is a first param
    :param param2: this is a second param
    :returns: this is a description of what is returned
    :raises keyError: raises an exception
    """
    pass

More styles here: https://stackoverflow.com/a/24385103

There are three separate standards.

Restructured
Numpy
Google

Using black shouldn't decide what integrations you're able to use.

@JelleZijlstra
Copy link
Collaborator

Black now has plenty opinions about docstrings (#1053) and this issue is long enough. If you feel that Black has the wrong opinion, or that it should have opinions about more aspects of docstrings, feel free to open a new issue.

@timokau
Copy link

timokau commented Sep 24, 2020

Should we re-open #745 then? The discussion was supposed to be moved here, but as far as I see no decision has been reached yet.

@bryevdv
Copy link

bryevdv commented Dec 31, 2020

Really wish I had seen this issue a few months ago. Lots of other tools process docstrings, and have their own expectations. black should stick to code formatting, and not be in the business of modifying docstrings in any way.

@chrisnorman7
Copy link

I've got a code base of some 10000 lines of code I'm maintaining on my own. I've just stumbled across the idea of autoformatting with black, and I'm fine with it's extra long line lengths. My only concern is that all my docstrings will remain < 80 characters per line.

I suppose this isn't really related to the issue at hand per se, but I definitely think that Black should be in the business of reformatting docstrings, although I'm not sure to what extent.

@PedanticHacker
Copy link

I personally prefer this docstring format:

class SimpleClass:
    """Create a simple class."""

    def __init__(self):
        """Initialize specific attributes for the simple class."""
        self.first_attribute = 1
        self.second_attribute = 2

    def sum_of_attributes(self):
        """
        Return the result of summing the values of this class' attributes.

        > Sum first_attribute and second_attribute.
        > Return the result of the summing calculation.
        """
        sum_calculation = self.first_attribute + self.second_attribute
        return sum_calculation

@ichard26 ichard26 removed help wanted Extra attention is needed good first issue Good for newcomers labels Jul 13, 2022
@gerbenoostra
Copy link

It seems Black isn't compatible with latest Docformatter (PyCQA/docformatter#94), because it removes the empty line after block docstrings. (Though also my personal preference would be to keep them)

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

No branches or pull requests