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

Doc string formatting behavior for multi-line comments #74

Closed
BrendanJM opened this issue Jan 17, 2024 · 8 comments
Closed

Doc string formatting behavior for multi-line comments #74

BrendanJM opened this issue Jan 17, 2024 · 8 comments

Comments

@BrendanJM
Copy link

Hi! Just learned about this library and love it so far - this is some beautiful command line tooling. I'm seeing some interesting behavior with docstrings. Let's take the sample app:

import cyclopts

app = cyclopts.App()


@app.command
def foo(name: str) -> None:
    """
    Do something. this is a multi-line
    comment wrapping over. words words words
    more words on more lines


    Args:
        name: something
    """
    print(f"Hello {name}!")


def main() -> None:
    app()


if __name__ == "__main__":
    main()

Running my_app foo -h, I see

$ my_app foo -h
Usage: my_app foo [ARGS] [OPTIONS]

Do something. this is a multi-line

comment wrapping over. words words words
more words on more lines

First, I have a slightly different docstring format than yours, but cool to see it just works for the most part. I am noticing that it looks like it generates a space after the first line. Is this intended behavior/is there any way to avoid injecting this space?

Or maybe this is just telling me I should be more concise :). Why waste time say lot word when few word do trick.

@BrianPugh
Copy link
Owner

BrianPugh commented Jan 17, 2024

Thanks for using Cyclopts!

The space between the Usage-string and the description is intentional. Currently there is no way of removing this newline, but if you're curious, it's injected right here.

The newlines in the long_description component of your docstring are a little less expected. This seems expected, however, according to the docstring_parser README. I would imagine an rst-like parsing would be ideal, and it seems like rich-rst would be the ticket, but I'm unsure about adding it as a dependency.

@BrianPugh
Copy link
Owner

ok, revisiting this, I now understand what you meant, you meant this space:

Do something. this is a multi-line

comment wrapping over. words words words

That is indeed unintentional, let me dig in.

@BrianPugh
Copy link
Owner

So diving in, the "problem" is kind of in parse_docstring.

In docstring_parser, they call inspect.cleandoc. The example input is (without the leading/trailing single-quote):

'\n    Do something. this is a multi-line\n    comment wrapping over. words words words\n    more words on more lines\n\n\n    Args:\n        name: something\n    '

and the output of the inspect.cleandoc call is:

'Do something. this is a multi-line\ncomment wrapping over. words words words\nmore words on more lines\n\n\nArgs:\n    name: something'

The documentation says to refer to PEP-0257. Here are some important relevent quotes:

The summary line may be used by automatic indexing tools; it is important that it fits on one line and is separated from the rest of the docstring by a blank line. The summary line may be on the same line as the opening quotes or on the next line.

The issue here, is your docstring doesn't have a "summary" (aka short_description) line, and so it interprets your first non-blank line as such. I think this is a bit erroneous behavior, as the line Do something. this is a multi-line doesn't have the mentioned blank-line.

So, if you format your docstring with the short_description, things will be formatted more as expected:

"""My function summary.

A multiline long description down here.
"""

My takeaway thoughts:

  1. It would be nice if the docstring body was rst-interpretted to make a prettier output.
  2. It would be nice if no summary-string is provided (delimited by the first \n\n), that the rest is interpreted as long_description.

@BrianPugh
Copy link
Owner

a workaround: I could do something like the following:

docstring = "" # Imagine this is your docstring

parsed = docstring_parse(raw_doc_string)  # call their docstring_parse

# Manual hacks
cleaned_docstring = inspect.doclean(docstring)
try:
    short_description = cleaned_docstring.split("\n\n")[0]
except IndexError:
    short_description = ""
    
if parsed.short_description != short_description:
    # Reconstruct the proper docstring
    parsed.long_description = parsed.short_description + "\n" + parsed.long_description
    # Get rid of the ill-parsed short_description
    parsed.short_description = None

# the rest of cyclopts help-page logic

Thoughts on something like this?

@BrianPugh
Copy link
Owner

I merged in some code into v2.1.1 that implements the spirit of the above. I still think functions should generally have a short description (they're used in the help-page for the command panel), but I think the current splitting was erroneous.

As for rst/md parsing, we can continue discussion in #81.

@BrendanJM
Copy link
Author

BrendanJM commented Jan 18, 2024

Wow, I appreciate the expeditious response and solution! Your solution here sounds great. One thing to note, but it looks like the short description gets parsed into the top level help command as well. In the case of the above:

$ my_app
╭─ Commands────────────────────────────────────────╮
│ foo        Do something. this is a multi-line    │
╰──────────────────────────────────────────────────╯
(

In the context of the proposed solution, clearing the short description looks like it might result in this being blank (unless I'm wrong). I'm not sure what the solution here would be. Options would seem to be:

  1. leave short description blank here (doesn't feel right)
  2. truncating the long description (e.g. after n characters Do something. this is a multi-line comment wrapping over. words .... Not super clean but not the worst.
  3. Put the full long description in here. If users find it too long, that's on them to shorten it up or break it into short + long.
  4. Go with to the original behavior of just taking the first line and probably just document what will happen and why you want a short + long description.

I think most of those other than option 1 would feel alright. Option 3 feels the most flexible to different documentation styles.

@BrianPugh
Copy link
Owner

BrianPugh commented Jan 18, 2024

Current behavior (as of v2.1.1) will be your (1):

  1. leave short description blank here (doesn't feel right)

(2) is a potential solution, but I don't think I want to deviate this far. Standard docstring syntax is:

"""My short summary.

A longer detailed description.
This is a continuation of that description.
"""

IMHO the current extension implemented in #82 is more consistent with PEP-0257, which is why I was ok with it. In short, I think it should be blank if there isn't a proper short_description. I don't think it's unreasonable to expect the user to provide a short_description/summary (standard practice) at the top of their docstring if they want the short_description field to be populated in the help-page.

@BrendanJM
Copy link
Author

I think going with whatever is most consistent with the PEP makes sense here. I'll align my documentation to avail of this. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants