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

Docstring description multiline parsing #476

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
made code more robust and added test cases (handling :)
  • Loading branch information
thebadcoder96 committed Jan 14, 2024
commit 5ce77266a3178b19147e914ac16d82a3791d8bc6
51 changes: 32 additions & 19 deletions fire/docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ def parse(docstring):
state.raises.lines = []
state.max_line_length = max(len(line) for line in lines)


for index, line in enumerate(lines):
has_next = index + 1 < lines_len
previous_line = lines[index - 1] if index > 0 else None
Expand Down Expand Up @@ -343,9 +342,10 @@ def _as_arg_name_and_type(text):
None otherwise.
"""
tokens = text.split()
is_type = any(c in "[](){}" for c in text)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason for this line?

Copy link
Author

@thebadcoder96 thebadcoder96 Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this, but if we add : after the first word in descriptions (line2 or later), the _is_arg_name() function will be called and according to our current logic, anything that is just a word will be considered as an arg. I think we want a better way to identify an arg.

This was the initial reason for this. I just came up with this for the time being but i think that we need a better way to identify an arg name.

if len(tokens) < 2:
return None
if _is_arg_name(tokens[0]):
if is_type and _is_arg_name(tokens[0]):
type_token = ' '.join(tokens[1:])
type_token = type_token.lstrip('{([').rstrip('])}')
return tokens[0], type_token
Expand Down Expand Up @@ -398,10 +398,8 @@ def _consume_google_args_line(line_info, state):
"""Consume a single line from a Google args section."""
split_line = line_info.remaining.split(':', 1)
if len(split_line) > 1:


first, second = split_line # first is either the "arg" or "arg (type)"
if _is_arg_name(first.strip()):
if _is_arg_name(first):
arg = _get_or_create_arg_by_name(state, first.strip())
arg.description.lines.append(second.strip())
arg.line1 = line_info.line
Expand All @@ -419,26 +417,41 @@ def _consume_google_args_line(line_info, state):
state.current_arg = arg
else:
if state.current_arg:
state.current_arg.description.lines.append(split_line[0])
state.current_arg.description.lines.append(':'.join(split_line))
_check_line2_line3(line_info, state)

else:
if state.current_arg:
state.current_arg.description.lines.append(split_line[0])
_check_line2_line3(line_info, state)

if line_info.previous.line == state.current_arg.line1: # check for line2
line2_first_word = line_info.line.strip().split(' ')[0]
state.current_arg.line2_first_word_length = len(line2_first_word)
state.current_arg.line2_length = len(line_info.line)
if line_info.next.line: #check for line3
line3_arg = len(line_info.next.line.split(':', 1)) > 1
if not line3_arg: #line3 should not be an arg
line3_first_word = line_info.next.line.strip().split(' ')[0]
state.current_arg.line3_first_word_length = len(line3_first_word)
else:
state.current_arg.line3_first_word_length = None

def _check_line2_line3(line_info, state):
"""Checks for line2 and line3, updating the arg states

Args:
line_info: information about the current line.
state: The state of the docstring parser.
"""
if line_info.previous.line == state.current_arg.line1: # check for line2
line2_first_word = line_info.line.strip().split(' ')[0]
state.current_arg.line2_first_word_length = len(line2_first_word)
state.current_arg.line2_length = len(line_info.line)
if line_info.next.line: #check for line3
line3_split = line_info.next.line.split(':', 1)
if len(line3_split) > 1:
line3_not_arg = not _is_arg_name(line3_split[0])
line3_not_type_arg = not _as_arg_name_and_type(line3_split[0])
else:
state.current_arg.line2_first_word_length = None
state.current_arg.line2_length = None
line3_not_arg = line3_not_type_arg = None
if line3_not_arg and line3_not_type_arg: #not an arg
line3_first_word = line_info.next.line.strip().split(' ')[0]
state.current_arg.line3_first_word_length = len(line3_first_word)
else:
state.current_arg.line3_first_word_length = None
else:
state.current_arg.line2_first_word_length = None
state.current_arg.line2_length = None


def _merge_if_long_arg(state):
Expand Down
80 changes: 80 additions & 0 deletions fire/docstrings_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,86 @@ def test_google_format_multiple_long_args_mixed_description(self):
)
self.assertEqual(expected_docstring_info, docstring_info)

def test_google_format_multiple_long_args_colon_description(self):
docstring = """This is a Google-style docstring with multiple long args.

Args:
param1_that_is_very_longer_than_usual: The first parameter. This has a lot of text,
enough to cover two lines.
param2_that_is_very_longer_than_usual: The second parameter. This has a lot of text,
enough to cover more than two lines: Maybe it can even go a lot more than
second line.
"""
docstring_info = docstrings.parse(docstring)
expected_docstring_info = DocstringInfo(
summary='This is a Google-style docstring with multiple long args.',
args=[
ArgInfo(name='param1_that_is_very_longer_than_usual',
description='The first parameter. This has a lot of text,'
' enough to cover two lines.'),
ArgInfo(name='param2_that_is_very_longer_than_usual',
description='The second parameter. This has a lot of text,'
' enough to cover more than two lines: Maybe it can even go a lot more'
' than\nsecond line.'),
],
)
self.assertEqual(expected_docstring_info, docstring_info)

def test_google_format_multiple_long_args_colons_description(self):
docstring = """This is a Google-style docstring with multiple long args.

Args:
param1_that_is_very_longer_than_usual: The first parameter. This has a lot of text,
enough to cover two lines.
param2_that_is_very_longer_than_usual: The second parameter. This has a lot of text,
enough to cover more than two lines: Maybe it can even go a lot more than
second line: Sometime the second line can be third too.
"""
docstring_info = docstrings.parse(docstring)
expected_docstring_info = DocstringInfo(
summary='This is a Google-style docstring with multiple long args.',
args=[
ArgInfo(name='param1_that_is_very_longer_than_usual',
description='The first parameter. This has a lot of text,'
' enough to cover two lines.'),
ArgInfo(name='param2_that_is_very_longer_than_usual',
description='The second parameter. This has a lot of text,'
' enough to cover more than two lines: Maybe it can even go a lot more'
' than\nsecond line: Sometime the second line can be third too.'),
],
)
self.assertEqual(expected_docstring_info, docstring_info)

def test_google_format_multiple_long_args_colons_overload(self):
docstring = """This is a Google-style docstring with multiple long args.

Args:
param1_that_is_very_longer_than_usual: The first parameter. This has a lot of text,
enough to cover: two lines.
param2_that_is_very_longer_than_usual: The second parameter. This has a lot of text,
enough to cover more than two lines: Maybe it can even go a lot more than
second line: Sometime the second line can be third too.
param3_that_is_very_longer_than_usual: The third parameter. This has a lot of text,
enough to: cover two lines.
"""
docstring_info = docstrings.parse(docstring)
expected_docstring_info = DocstringInfo(
summary='This is a Google-style docstring with multiple long args.',
args=[
ArgInfo(name='param1_that_is_very_longer_than_usual',
description='The first parameter. This has a lot of text,'
' enough to cover: two lines.'),
ArgInfo(name='param2_that_is_very_longer_than_usual',
description='The second parameter. This has a lot of text,'
' enough to cover more than two lines: Maybe it can even go a lot more'
' than\nsecond line: Sometime the second line can be third too.'),
ArgInfo(name='param3_that_is_very_longer_than_usual',
description='The third parameter. This has a lot of text,'
' enough to: cover two lines.'),
],
)
self.assertEqual(expected_docstring_info, docstring_info)

def test_rst_format_typed_args_and_returns(self):
docstring = """Docstring summary.

Expand Down