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

Multi-line parameter descriptions in Google-style docstrings #261

Closed
lukedeo opened this issue May 21, 2020 · 12 comments
Closed

Multi-line parameter descriptions in Google-style docstrings #261

lukedeo opened this issue May 21, 2020 · 12 comments

Comments

@lukedeo
Copy link

lukedeo commented May 21, 2020

Hey there! Quick question regarding the behavior of multi-line parameter descriptions in Google-style docstrings.

Consider the following code:

# main.py
def foo(bar):
    """
    This does a foo on a bar.

    Args:
        bar (str): Though a simple parameter, this parameter will need a lot of
            verbiage to describe, requiring a second line.
    """
    print(f"Hi, {bar}!")

if __name__ == "__main__":
    import fire

    fire.Fire(foo)

When running main.py --help, the resulting man is:

NAME
    main.py - This does a foo on a bar.

SYNOPSIS
    main.py BAR

DESCRIPTION
    This does a foo on a bar.

POSITIONAL ARGUMENTS
    BAR
        Though a simple parameter, this parameter will need a lot of

NOTES
    You can also use flags syntax for POSITIONAL ARGUMENTS

Notice that we are losing the following line here.

I'd expect the newline-containing description to be un-newlined (definitely not a word)

NAME
    main.py - This does a foo on a bar.

SYNOPSIS
    main.py BAR

DESCRIPTION
    This does a foo on a bar.

POSITIONAL ARGUMENTS
    BAR
        Though a simple parameter, this parameter will need a lot of verbiage to describe, requiring a second line.

NOTES
    You can also use flags syntax for POSITIONAL ARGUMENTS

Or maybe included with the newline, but in some way retaining the following line. Any plans to support this, or (likely) am I missing something?

@lukedeo lukedeo changed the title Google-style docstrings with multiple lines. Multi-line parameter descriptions in Google-style docstrings May 21, 2020
@dbieber
Copy link
Member

dbieber commented May 21, 2020

I actually thought we supported this already - I'll mark it as a bug. Yes, we would like for this to work.

@lukedeo
Copy link
Author

lukedeo commented May 21, 2020

Thanks @dbieber!

@MichaelCG8
Copy link
Contributor

I've seen the same using Numpy style docstrings, I'll find an example and add it later. I'll look into this tonight and see if it's something I can help with.

@MichaelCG8
Copy link
Contributor

I've experimented and found the following.

def foo(bar):
"""
This does a foo on a bar.

Args:
    bar (str): Though a simple parameter, this parameter will need a lot of
        verbiage to describe, requiring a second line.
"""
print(f"Hi, {bar}!")

def foo2(bar):
  """
  This does a foo on a bar.

  Parameters
  ----------
  bar : str
      Though a simple parameter, this parameter will need a lot of
      verbiage to describe, requiring a second line.
  """

def foo3(bar):
  """
  This does a foo on a bar.

  Parameters
  ----------
  bar
    With Numpy style strings, this fails. It is strange, since it doesn't fail for foo2.
    Missing.
  """
  print(f"Hi, {bar}!")

def foo4(bar):
  """
  This does a foo on a bar.

  Parameters
  ----------
  bar
    With Numpy style strings, this fails. It is strange, since it doesn't fail for foo2.
    Missing line here.
  """
  print(f"Hi, {bar}!")

if __name__ == "__main__":
  import fire

  fire.Fire()

I access the help text using python -m bug_replication foo --help

foo is as @lukedeo described.
foo2 is a Numpy style equivalent, it renders fine.
foo3 The second line does not appear.
foo4 The second line does appear.

@MichaelCG8
Copy link
Contributor

Okay, there appears to be two issues.

The issue with foo3 is that in Numpy docstrings if a line in an arg description is interpreted as the next argument if it does not contain a space or a colon. See fire.docstrings.is_arg_name(). I've updated this to include checking for a period. It would probably be more elegant to do something based on the indentation, but this is simple and fixes the issue.

The issue with foo is different. In Google docstrings if the argument name is not accompanied by a type then fire.docstrings._consume_google_args_line() records the current arg with the line state.current_arg = arg. This is missing from the case where an argument name is accompanied by a type. If you remove the type frombar in foo's docstring, then the missing part of the line is displayed correctly.

I've put the fix in a fork here https://github.com/MichaelCG8/python-fire/tree/issue-261-bugfix-multi-line-docstring-parameter-descriptions so feel free to have a look at that.

Next I'll add some tests for these cases, then I'll open a pull request.

@MichaelCG8
Copy link
Contributor

Added the fix in pull request #262.
For the Numpy issue I opted for a regex comparison for a valid argument name, since it fixes more issues than just my example by checking for a period.

@dbieber I was wondering if you have a feel for the release schedule at the moment?

@dbieber
Copy link
Member

dbieber commented May 23, 2020

Thanks for the fix!

The next feature-ful release doesn't have a date or ETA yet. The two primary goals for that are to 1) enable configs similar to what is discussed in #188 (display, serialize), and 2) enable a strict type mode, allowing Fire to benefit from type hints and prevent accidental type mismatches (such as passing an int to a function that expects a string).

We can plan smaller releases before then on an as-needed basis, such as to release bug fixes like this one. The time from bug-fix to merge to release might be a few weeks (this is a high variance, rough estimate) depending on how busy @joejoevictor and I are.

@MichaelCG8
Copy link
Contributor

Hi, thanks for the info! Number 2 sounds interesting, is there an existing issue open for that?

@dbieber
Copy link
Member

dbieber commented May 26, 2020

#226

@orkun1675
Copy link

We have been avid users of Fire (thank you for making it available!) and recently ran into this issue as well.

@MichaelCG8 with your PR would the following work as well?

  def foo(self, name):
    """Greets name.

    Arguments:
      name: The person to greet. This is a very long line that has to be word
        wrapped and contains a colon (:) in it.
    """
    print('Hello %s' % name)

@MichaelCG8
Copy link
Contributor

@orkun1675 No, my PR doesn't fix that case, but I can see the cause. I've opened #309 to track.

@dbieber
Copy link
Member

dbieber commented Jan 22, 2021

Done in #262
Thanks for opening #309 and for the fix here, @MichaelCG8

@dbieber dbieber closed this as completed Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants