D417 - Add hanging indent support for Google style

[ColinKennedy]

Following the existing conversation seen here (#443)

This issue is to request that hanging indentation documentation be added to Google style.

Args

Currently this style is recommended by pydocstyle:

    Args:
        verbose (bool): If True, print out as much information as possible.
            If False, print out concise "one-liner" information.

And this issue is to request that this other style also be valid:

    Args:
        verbose (bool): 
            If True, print out as much information as possible.
            If False, print out concise "one-liner" information.

@samj1912's link to Google-style, http://google.github.io/styleguide/pyguide.html#383-functions-and-methods, shows that this is already allowed in Google style.

Returns / Yields

The Google style guide explicitly mentions newlines being allowed for args. I'd like to further propose that the same apply for Return docstrings.

For example, this

    Returns
        :obj:`list` of :obj:`tuple` str or :class:`my_package.modules.MyKlass`: Line already exceeds 0 characters.

Can become this

    Returns
        :obj:`list` of :obj:`tuple` str or :class:`my_package.modules.MyKlass`: 
            Each line is within 80 characters.

To the maintainers of this repo - would you accept a PR to add this feature?

[samj1912]

I am sorry, I think you misunderstood me.

Copying from Google style guide -

. A description should follow the name, and be separated by a colon and a space. If the description is too long to fit on a single 80-character line, use a hanging indent of 2 or 4 spaces (be consistent with the rest of the file).

http://google.github.io/styleguide/pyguide.html#383-functions-and-methods

See the part in bold. It is supposed to follow the name and be separated by a colon and space.
Not start from a new line. The hanging indent part applies to the remaining description.

There are examples of multi line doc strings in the style guide and all of them start on the same line.

[ColinKennedy]

I read "If the description is too long to fit on a single 80-character line, use a hanging indent of 2 or 4 spaces (be consistent with the rest of the file)." as in "If it's too long, it doesn't need to start on the same follow the name". It didn't occur to me that it may be been referring to overflow.

That said, the original purpose of this post still stands. I think it's a worthy addition. What do others think?

[samj1912]

I would just like to add that napoleon recommends the current pydocstyle format as well, with the description starting from the same line.

See the example in module level function.

https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google

The example states -

The format for a parameter is::

    name (type): description
        The description may span multiple lines. Following
        lines should be indented. The "(type)" is optional.

        Multiple paragraphs are supported in parameter
        descriptions.

Args:
    param1 (int): The first parameter.
    param2 (:obj:`str`, optional): The second parameter. Defaults to None.
        Second line of description should be indented.
    *args: Variable length argument list.
    **kwargs: Arbitrary keyword arguments.

The format is supposed to be
name (type): description
With the description starting on the same line as the argument name (see parts in bold). None of the multi line examples either in Google style guide or napoleon (the most popular tool for parsing them) examples support the new line/hanging format as proposed in the original issue, nor do the guidelines say anything about using it.

I wouldn't recommend we change the checks in pydocstyle when there is no evidence for the above style anywhere.

The only argument for including is that it makes it more readable when the type annotations are long, which, given that Google recommends using pep 484 style annotations, should ideally never be so long (you should be using meaningful type aliases). It is also more useful to have the annotations in the function definition rather than the docstring, but that is another conversation.

[martinlanton]

It seems to me that the only argument NOT to do that is because it is not supported by other tools and is not mentioned anywhere else, whereas @ColinKennedy seems to make a valid point in the fact that having the description start on the second line in the case of multi line descriptions with multiple arguments makes it much easier to read.

As he mentioned in the other thread ( #443 ), this :

def foo(something=[(8, 1.0), (-1, 11.0), (3, 4, 6, 7)], another_argument=frozenset(), more=None):
    """Do something.

    Args:
        something (iter[tuple[int, float] or tuple[int]], optional): 
            My information.
        another_argument (set[str], optional): 
            Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut
            erat urna, finibus non tincidunt scelerisque, commodo eget ante.
        more (None, optional): 
           Aliquam vel justo convallis est sollicitudin egestas et quis
           arcu. Nam ut mauris luctus, elementum dolor eu, placerat
           turpis. Ut non consequat augue, sit amet dapibus arcu.
           Vivamus mattis dui vel mi condimentum, pellente

    """

is much easier to read than this :

def foo(something=[(8, 1.0), (-1, 11.0), (3, 4, 6, 7)], another_argument=frozenset(), more=None):
    """Do something.

    Args:
        something (iter[tuple[int, float] or tuple[int]], optional): My information.
        another_argument (set[str], optional): Lorem ipsum dolor sit amet, consectetur 
            adipiscing elit. Ut erat urna, finibus non tincidunt scelerisque, commodo eget ante. 
        more (None, optional): Aliquam vel justo convallis est sollicitudin egestas et quis arcu. 
           Nam ut mauris luctus, elementum dolor eu, placerat turpis. Ut non consequat augue, 
           sit amet dapibus arcu. Vivamus mattis dui vel mi condimentum, pellente
    """

And as much as I prefer the first example for single line description, I think it'd be a good idea to support the second example for multi lines descriptions.

[samj1912]

It seems to me that the only argument NOT to do that is because it is not supported by other tools and is not mentioned anywhere else

That's a pretty strong argument. IMHO Pydocstyle is not the right place to create new rules or change existing ones. It's first and foremost a linter to enforce the existing rules. The whole point of having a convention and sticking to it is to avoid having pointless discussions on which style is better (the approach that black takes).

Let me be clear, the proposed one might be more readable for this specific example, but if we allow for it to be a valid docstring, we will also be opening a can of worms with regards to style inconsistencies in existing projects. To be clear what you are proposing would allow the following to be valid. I do not want a project to have -

def foo(something=[(8, 1.0), (-1, 11.0), (3, 4, 6, 7)], something_else=None, another_argument=frozenset(), more=None):
    """Do something.

    Args:
        something: 
            My information.
        something_else: Not my information. 
        another_argument: 
            Lorem ipsum dolor sit amet, consectetur 
            adipiscing elit. Ut erat urna, finibus non tincidunt scelerisque, commodo eget ante. 
        more: Aliquam vel justo convallis est sollicitudin egestas et quis arcu. 
           Nam ut mauris luctus, elementum dolor eu, placerat turpis. Ut non consequat augue, 
           sit amet dapibus arcu. Vivamus mattis dui vel mi condimentum, pellente
    """

The above looks extremely inconsistent.

I would also like to mention that Google style guide explicitly mentions the format of the docstring -

The format is supposed to be
name (type): description
With the description starting on the same line as the argument name

The proposed change seems to clearly go against all of this.

[samj1912]

IMHO the correct platform to raise this issue or have this discussion is https://github.com/google/styleguide/issues

[ColinKennedy]

The only argument for including is that it makes it more readable when the type annotations are long, which, given that Google recommends using pep 484 style annotations, should ideally never be so long (you should be using meaningful type aliases).

With or without types included, it's still an issue.

Examples:

        """Keep track of the current Rez package and the user's settings.

        Args:
            package: The current Rez package that is being checked.
            processed_packages: Other install Rez packages that have
                already been processed. These packages should be
                pre-validated and are meant only for caching. Default is None.
            allow_vimgrep: Some user input to track that specifies if they want row/column data printed.
            verbose: Some user input to track that specifies
                if they want summary information or everything to be
                printed.

        """

Compared to...

        """Keep track of the current Rez package and the user's settings.

        Args:
            package:
                The current Rez package that is being checked.
            processed_packages:
                Other install Rez packages that have already been
                processed. These packages should be pre-validated and
                are meant only for caching. Default is None.
            allow_vimgrep:
                Some user input to track that specifies if they want
                row/column data printed.
            verbose:
                Some user input to track that specifies if they want
                summary information or everything to be printed.

        """

Having descriptions start on the first line makes it hard for users to know when to split from the first line into the second. And it prevents auto-formatting tools like par from being able to auto-align the text to a single column count.

I do not want a project to have - ...

The current situation regarding whitespace / indentation is already inconsistent. (see package followed by processed_packages followed by allow_vimgrep).

I'll raise with the link you've suggested.

[ColinKennedy]

google/styleguide#505

We've got a custom pylint GoogleDocstringChecker internally that verifies the our style of docstring format. My internal response to this kind of question is generally to defer to whatever our lint checker implementation asks for when it comes to details like this. Running your final example through that, it likes it. Which is a strong indicator that we allow that nice readable form. :)

Looks like "next-line" style is allowed. Since that is the case, can we go forward with adding this to pydocstyle? Would you accept a PR with this change?

[rmorshea]

Any movement on this? I'm eagerly awaiting this issue's resolution.

While there are cases where the single-line style is nice and succinct, if I had to choose one, I'd want to enforce the hanging-indent style, and as others have mentioned, supporting one option rather than both is probably better, both from an options perspective, and from a stylistic consistency perspective.

[ColinKennedy]

It's a hard call. I guess the questions come down to

• Should pydocstyle support both standards at once
  • If it does support both standards, does it allow mixing within the same docstring or should it force you to use maintain style or the other (for consistency)
• Should pydocstyle support a single standard

If the answer is "there can only be one" then yes, I'd highly recommend the hanging-indent style for the reasons previously mentioned. But it's not my call. That said, I'm also eager to address this too, since I think D417 has a ton of value and I want to use it as much as possible.

For pydocstyle, is indentation-consistency a worthy requirement? Or is mixing permissible? Let's answer that first and move forward from there. Answering that question first will help determine a refactor.

[rmorshea]

@ColinKennedy IMO consistency is king when it comes to readability - in most cases bad style done consistently is better than individually pleasant styles that have been mixed. In this particular case though, I think we could potentially allow mixed styles between different docstrings, but not within the same docstring.

[rmorshea]

With that said, I don't think that we should allow multiline descriptions that do not use the hanging indent. The two allowed styles across a codebase ought to to be:

• single line descriptions for all parameters (maybe with a line length check?)
• hanging indent for all parameters

ALLOWED

Args:
    param1: Ut erat urna, finibus non tincidunt scelerisque
    param2: Non tincidunt hubtick Ut erat urna, finibus

ALLOWED

Args:
    param1:
        Ut erat urna, finibus non tincidunt scelerisque
    param2:
        Non tincidunt hubtick Ut erat urna, finibus

DISALLOWED (mixed style)

Args:
    param1: Ut erat urna, finibus non tincidunt scelerisque
    param2:
        Non tincidunt hubtick Ut erat urna, finibus

DISALLOWED (multiline without hanging indent)

Args:
    param1: Hubtick Ut erat hoptibwook zacknut ubner
        ut erat urna, finibus non tincidunt scelerisque
    param2: Tooberul stunbynt all unt moov tacklobn i na
        on tincidunt hubtick Ut erat urna, finibus

[ColinKennedy]

DISALLOWED (multiline without hanging indent)

The google style guide allows this. This issue is about supporting (in pydocstyle) what google style already supports. So we shouldn't be trying to disallow something that the google style allows.

DISALLOWED (mixed style)

Honestly I don't mind if users do mix. But if the maintainers here strongly wish to keep things uniform, IMO your previous note "potentially allow mixed styles between different docstrings, but not within the same docstring" is probably the way to go.

@samj1912 or @Nurdok what do you think about @rmorshea 's proposal? Would you like to take it as-is or make any adjustments?

[samj1912]

I'm still a bit undecided about this whole affair. Can we do the following -

Make a PR upstream to Google style guide to add the hanging indent style docstrings, just so everyone is aware that's a valid way to document arguments using Google style. Ask the same questions upstream on what they think about the above proposal. Once the Style guide is updated, we simply follow what it proposes. That way, we avoid such changes being introduced via pydocstyle.

My main concern about introducing this in pydocstyle is that the only place where we have any confirmation that the hanging style is valid, is a issue on the repo. There is no visible documentation confirming the same.

Once we have the above, I'm more than happy to implement the changes to support the new indent style myself.

Again, I would like to point out what I've iterated previously. Pydocstyle is simply a linter that enforces existing prominent styles. It is not a place to debate or decide modifications to the same.

[samj1912]

By upstream, I mean making changes to https://github.com/google/styleguide/ to add the hanging indent examples.

[ColinKennedy]

I can try but I don't know what google's PR process is like.

The important distinction, IMO, is back in the style guide

"A description should follow the name, and be separated by a colon and a space."

If that were a hard requirement then the earlier test using Google's internal checker would not have passed while starting from a new, indented line. It does pass, which means Google's style guide has at least a bit of flexibility.

That said, I'll try to make a PR later today in the evening.

[ColinKennedy]

@samj1912

Google Python Style Guide has been updated to allow newlines (Reference: https://google.github.io/styleguide/pyguide.html#doc-function-args)

I'd like to revisit this issue and add newline support for D417, assuming it isn't already.

[samj1912]

@ColinKennedy sgtm - please go ahead and create a PR. Just make sure that as the docs say - the style is consistent with the rest of the docstrings in the file :).

[JMMarchant]

Has there been any further movement on this?

[ColinKennedy]

Not on my side. Things got too busy for me since the last update. Not sure if others took this on or if the proposed change is still acceptable for PR.

[rmorshea]

I put up a PR to solve this: #564

[rmorshea]

I think #564 is close to completion. Just awaiting final review now.