From 45e3a0e7be335b33fd83b24b06ba8fdc1baee884 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Wed, 20 Dec 2023 23:16:14 -0600 Subject: [PATCH 01/11] quick fix --- fire/docstrings.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 1cfadea9..990a6b3b 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -200,7 +200,7 @@ def parse(docstring): args.extend([KwargInfo( name=arg.name, type=_cast_to_known_type(_join_lines(arg.type.lines)), - description=_join_lines(arg.description.lines)) for arg in state.kwargs]) + description=_join_lines(arg.description.lines, 'description')) for arg in state.kwargs]) return DocstringInfo( summary=summary, @@ -239,7 +239,7 @@ def _is_blank(line): return not line or line.isspace() -def _join_lines(lines): +def _join_lines(lines, type=None): """Joins lines with the appropriate connective whitespace. This puts a single space between consecutive lines, unless there's a blank @@ -253,7 +253,7 @@ def _join_lines(lines): # TODO(dbieber): Add parameters for variations in whitespace handling. if not lines: return None - + started = False group_texts = [] # Full text of each section. group_lines = [] # Lines within the current section. @@ -269,7 +269,12 @@ def _join_lines(lines): group_lines = [] if group_lines: # Process the final group. - group_text = ' '.join(group_lines) + # group_text = ' '.join(group_lines) + if type == 'description': + group_text = '\n'.join(group_lines) + else: + group_text = ' '.join(group_lines) + group_texts.append(group_text) return '\n\n'.join(group_texts) From b395e20995404a0c767294797405ad3d5881c7c2 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Fri, 22 Dec 2023 22:22:02 -0600 Subject: [PATCH 02/11] Added '\n' to _join_lines() and updated tests --- fire/docstrings.py | 7 +------ fire/docstrings_test.py | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 990a6b3b..3ec8ce1d 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -269,12 +269,7 @@ def _join_lines(lines, type=None): group_lines = [] if group_lines: # Process the final group. - # group_text = ' '.join(group_lines) - if type == 'description': - group_text = '\n'.join(group_lines) - else: - group_text = ' '.join(group_lines) - + group_text = '\n'.join(group_lines) group_texts.append(group_text) return '\n\n'.join(group_texts) diff --git a/fire/docstrings_test.py b/fire/docstrings_test.py index 0d6e5d18..99034d31 100644 --- a/fire/docstrings_test.py +++ b/fire/docstrings_test.py @@ -165,7 +165,7 @@ def test_google_format_multiline_arg_description(self): description='The first parameter.'), ArgInfo(name='param2', type='str', description='The second parameter. This has a lot of text, ' - 'enough to cover two lines.'), + 'enough to\ncover two lines.'), ], ) self.assertEqual(expected_docstring_info, docstring_info) @@ -229,7 +229,7 @@ def test_numpy_format_typed_args_and_returns(self): description='The second parameter.'), ], # TODO(dbieber): Support return type. - returns='bool True if successful, False otherwise.', + returns='bool\nTrue if successful, False otherwise.', ) self.assertEqual(expected_docstring_info, docstring_info) @@ -257,7 +257,7 @@ def test_numpy_format_multiline_arg_description(self): description='The first parameter.'), ArgInfo(name='param2', type='str', description='The second parameter. This has a lot of text, ' - 'enough to cover two lines.'), + 'enough to cover two\nlines.'), ], ) self.assertEqual(expected_docstring_info, docstring_info) From 6f0ce182e17260b0b2237da94f5dd8053c1ca70c Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Fri, 22 Dec 2023 22:26:14 -0600 Subject: [PATCH 03/11] cleaned py file --- fire/docstrings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 3ec8ce1d..34c03533 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -200,7 +200,7 @@ def parse(docstring): args.extend([KwargInfo( name=arg.name, type=_cast_to_known_type(_join_lines(arg.type.lines)), - description=_join_lines(arg.description.lines, 'description')) for arg in state.kwargs]) + description=_join_lines(arg.description.lines)) for arg in state.kwargs]) return DocstringInfo( summary=summary, @@ -239,7 +239,7 @@ def _is_blank(line): return not line or line.isspace() -def _join_lines(lines, type=None): +def _join_lines(lines): """Joins lines with the appropriate connective whitespace. This puts a single space between consecutive lines, unless there's a blank From 4b770d97b8863b5015877899d9e1e7f2c1553eae Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Tue, 2 Jan 2024 10:27:23 -0600 Subject: [PATCH 04/11] Added changes based on discussion --- fire/docstrings.py | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/fire/docstrings.py b/fire/docstrings.py index 34c03533..ac0dd07d 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -177,6 +177,12 @@ def parse(docstring): state.returns.lines = [] state.yields.lines = [] state.raises.lines = [] + state.line1 = None + state.line1_length = None + state.line2_first_word_length = None + state.line2_length = None + state.line3_first_word_length = None + state.max_line_length = 0 for index, line in enumerate(lines): has_next = index + 1 < lines_len @@ -184,6 +190,9 @@ def parse(docstring): next_line = lines[index + 1] if has_next else None line_info = _create_line_info(line, next_line, previous_line) _consume_line(line_info, state) + + if state.line2_length: + _merge_if_long_arg(state) summary = ' '.join(state.summary.lines) if state.summary.lines else None state.description.lines = _strip_blank_lines(state.description.lines) @@ -391,6 +400,9 @@ 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: + state.line1 = line_info.line + state.line1_length = len(line_info.line) + state.max_line_length = max(state.max_line_length, state.line1_length) first, second = split_line # first is either the "arg" or "arg (type)" if _is_arg_name(first.strip()): arg = _get_or_create_arg_by_name(state, first.strip()) @@ -410,6 +422,65 @@ def _consume_google_args_line(line_info, state): else: if state.current_arg: state.current_arg.description.lines.append(split_line[0]) + state.max_line_length = max(state.max_line_length, len(line_info.line)) + if line_info.previous.line == state.line1: # check for line2 + state.line2_first_word_length = len(line_info.line.strip().split(' ')[0]) + state.line2_length = len(line_info.line) + if line_info.next.line: #check for line3 + state.line3_first_word_length = len(line_info.next.line.strip().split(' ')[0]) + + +def _merge_if_long_arg(state): + """Merges first two lines of the description if the arg name is too long. + + Args: + state: The state of the docstring parser. + """ + apparent_max_line_length = roundup(state.max_line_length) + long_arg_name = roundup(len(state.current_arg.name), 5) >= 0.5 * apparent_max_line_length + if long_arg_name and state.line2_first_word_length and state.line3_first_word_length: + line1_intentionally_short = (state.line1_length + state.line2_first_word_length) <= apparent_max_line_length + line2_intentionally_short = (state.line2_length + state.line3_first_word_length) <= apparent_max_line_length + line1_intentionally_long = state.line1_length >= 1.05 * apparent_max_line_length + line2_intentionally_long = state.line2_length >= 1.05 * apparent_max_line_length + if not line1_intentionally_short and not line1_intentionally_long and not line2_intentionally_short and not line2_intentionally_long: + _merge_line1_line2(state.current_arg.description.lines) + + +def _merge_line1_line2(lines): + """Merges the first two lines of a list of strings. + + Example: + _merge_line1_line2(["oh","no","bro"]) == ["oh no","bro"] + + Args: + lines: a list of strings representing each line. + Returns: + the same list but with the first two lines of the list now merged as one line. + """ + merged_line = lines[0] + " " + lines[1] + lines[0] = merged_line + lines.pop(1) + return lines + + +def roundup(number, multiple=10): + """Rounds a number to the nearst multiple. + + Example: + roundup(72) == 80 + + Args: + number: an interger type variable. + multiple: nearst multiple to round up to + Returns: + An interger value. + """ + remainder = number % multiple + if remainder == 0: + return number #already rounded + else: + return number + (multiple - remainder) def _consume_line(line_info, state): From 93ff1118f10aea460f761662feea4d6b25f24739 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Tue, 2 Jan 2024 13:23:40 -0600 Subject: [PATCH 05/11] updated code to resolved comment --- fire/docstrings.py | 49 ++++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index ac0dd07d..e19c117f 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -190,9 +190,6 @@ def parse(docstring): next_line = lines[index + 1] if has_next else None line_info = _create_line_info(line, next_line, previous_line) _consume_line(line_info, state) - - if state.line2_length: - _merge_if_long_arg(state) summary = ' '.join(state.summary.lines) if state.summary.lines else None state.description.lines = _strip_blank_lines(state.description.lines) @@ -262,7 +259,7 @@ def _join_lines(lines): # TODO(dbieber): Add parameters for variations in whitespace handling. if not lines: return None - + started = False group_texts = [] # Full text of each section. group_lines = [] # Lines within the current section. @@ -424,11 +421,17 @@ def _consume_google_args_line(line_info, state): state.current_arg.description.lines.append(split_line[0]) state.max_line_length = max(state.max_line_length, len(line_info.line)) if line_info.previous.line == state.line1: # check for line2 - state.line2_first_word_length = len(line_info.line.strip().split(' ')[0]) + line2_first_word = line_info.line.strip().split(' ')[0] + state.line2_first_word_length = len(line2_first_word) state.line2_length = len(line_info.line) if line_info.next.line: #check for line3 - state.line3_first_word_length = len(line_info.next.line.strip().split(' ')[0]) - + line3_first_word = line_info.next.line.strip().split(' ')[0] + state.line3_first_word_length = len(line3_first_word) + else: + state.line3_first_word_length = None + else: + state.line2_first_word_length = state.line2_length = None + def _merge_if_long_arg(state): """Merges first two lines of the description if the arg name is too long. @@ -436,15 +439,25 @@ def _merge_if_long_arg(state): Args: state: The state of the docstring parser. """ - apparent_max_line_length = roundup(state.max_line_length) - long_arg_name = roundup(len(state.current_arg.name), 5) >= 0.5 * apparent_max_line_length - if long_arg_name and state.line2_first_word_length and state.line3_first_word_length: - line1_intentionally_short = (state.line1_length + state.line2_first_word_length) <= apparent_max_line_length - line2_intentionally_short = (state.line2_length + state.line3_first_word_length) <= apparent_max_line_length - line1_intentionally_long = state.line1_length >= 1.05 * apparent_max_line_length - line2_intentionally_long = state.line2_length >= 1.05 * apparent_max_line_length - if not line1_intentionally_short and not line1_intentionally_long and not line2_intentionally_short and not line2_intentionally_long: - _merge_line1_line2(state.current_arg.description.lines) + actual_max_line_len = roundup(state.max_line_length) + arg_length = len(state.current_arg.name) + percent_105 = 1.05 * actual_max_line_len + long_arg_name = roundup(arg_length, 5) >= 0.5 * actual_max_line_len + if long_arg_name: + if state.line2_first_word_length: + line1_plus_first_word = state.line1_length + state.line2_first_word_length + line1_intentionally_short = line1_plus_first_word <= actual_max_line_len + line1_intentionally_long = state.line1_length >= percent_105 + line2_intentionally_long = state.line2_length >= percent_105 + if state.line3_first_word_length: + line2_plus_first_word = state.line2_length + state.line3_first_word_length + line2_intentionally_short = line2_plus_first_word <= actual_max_line_len + if not line1_intentionally_short and not line1_intentionally_long: + if not line2_intentionally_short and not line2_intentionally_long: + _merge_line1_line2(state.current_arg.description.lines) + elif not line1_intentionally_short and not line1_intentionally_long: + if not line2_intentionally_long: + _merge_line1_line2(state.current_arg.description.lines) def _merge_line1_line2(lines): @@ -456,7 +469,7 @@ def _merge_line1_line2(lines): Args: lines: a list of strings representing each line. Returns: - the same list but with the first two lines of the list now merged as one line. + same list but with the first two lines of the list now merged as a line. """ merged_line = lines[0] + " " + lines[1] lines[0] = merged_line @@ -536,6 +549,8 @@ def _consume_line(line_info, state): if state.section.title == Sections.ARGS: if state.section.format == Formats.GOOGLE: _consume_google_args_line(line_info, state) + if state.current_arg and line_info.previous.line == state.line1: + _merge_if_long_arg(state) elif state.section.format == Formats.RST: state.current_arg.description.lines.append(line_info.remaining.strip()) elif state.section.format == Formats.NUMPY: From 645c51d66493ec2fd6643dc3ae78fe033196468d Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Tue, 2 Jan 2024 14:18:26 -0600 Subject: [PATCH 06/11] Changed threshold to 40% --- fire/docstrings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index e19c117f..926fb8c2 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -442,7 +442,7 @@ def _merge_if_long_arg(state): actual_max_line_len = roundup(state.max_line_length) arg_length = len(state.current_arg.name) percent_105 = 1.05 * actual_max_line_len - long_arg_name = roundup(arg_length, 5) >= 0.5 * actual_max_line_len + long_arg_name = roundup(arg_length, 5) >= 0.4 * actual_max_line_len if long_arg_name: if state.line2_first_word_length: line1_plus_first_word = state.line1_length + state.line2_first_word_length From c56d951098ef1a9c13836c1aa304d30ba3d67cf6 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Tue, 2 Jan 2024 18:19:33 -0600 Subject: [PATCH 07/11] Added test cases --- fire/docstrings.py | 12 ++-- fire/docstrings_test.py | 121 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 926fb8c2..7bb11ed2 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -423,10 +423,12 @@ def _consume_google_args_line(line_info, state): if line_info.previous.line == state.line1: # check for line2 line2_first_word = line_info.line.strip().split(' ')[0] state.line2_first_word_length = len(line2_first_word) - state.line2_length = len(line_info.line) + state.line2_length = len(line_info.line) if line_info.next.line: #check for line3 - line3_first_word = line_info.next.line.strip().split(' ')[0] - state.line3_first_word_length = len(line3_first_word) + 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.line3_first_word_length = len(line3_first_word) else: state.line3_first_word_length = None else: @@ -446,12 +448,12 @@ def _merge_if_long_arg(state): if long_arg_name: if state.line2_first_word_length: line1_plus_first_word = state.line1_length + state.line2_first_word_length - line1_intentionally_short = line1_plus_first_word <= actual_max_line_len + line1_intentionally_short = roundup(line1_plus_first_word) < actual_max_line_len line1_intentionally_long = state.line1_length >= percent_105 line2_intentionally_long = state.line2_length >= percent_105 if state.line3_first_word_length: line2_plus_first_word = state.line2_length + state.line3_first_word_length - line2_intentionally_short = line2_plus_first_word <= actual_max_line_len + line2_intentionally_short = roundup(line2_plus_first_word) < actual_max_line_len if not line1_intentionally_short and not line1_intentionally_long: if not line2_intentionally_short and not line2_intentionally_long: _merge_line1_line2(state.current_arg.description.lines) diff --git a/fire/docstrings_test.py b/fire/docstrings_test.py index 99034d31..cba26ab7 100644 --- a/fire/docstrings_test.py +++ b/fire/docstrings_test.py @@ -170,6 +170,127 @@ def test_google_format_multiline_arg_description(self): ) self.assertEqual(expected_docstring_info, docstring_info) + def test_google_format_long_arg_long_description(self): + docstring = """This is a Google-style docstring with long args. + + Args: + function_maker_handler_event_factory: The function-maker-handler event factory + responsible for making the function-maker-handler event. Use this whenever + you need a function-maker-handler event made for you programmatically. + """ + docstring_info = docstrings.parse(docstring) + expected_docstring_info = DocstringInfo( + summary='This is a Google-style docstring with long args.', + args=[ + ArgInfo(name='function_maker_handler_event_factory', + description='The function-maker-handler event factory ' + 'responsible for making the function-maker-handler event. ' + 'Use this whenever\nyou need a function-maker-handler event' + ' made for you programmatically.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + + def test_google_format_multiple_long_args(self): + docstring = """This is a Google-style docstring with multiple long args. + + Args: + function_maker_handler_event_factory: The function-maker-handler event factory + responsible for making the function-maker-handler event. Use this whenever + you need a function-maker-handler event made for you programmatically. + function_maker_handler_event_factory2: The function-maker-handler event factory + responsible for making the function-maker-handler event. Use this whenever + you need a function-maker-handler event made for you programmatically. + """ + docstring_info = docstrings.parse(docstring) + expected_docstring_info = DocstringInfo( + summary='This is a Google-style docstring with multiple long args.', + args=[ + ArgInfo(name='function_maker_handler_event_factory', + description='The function-maker-handler event factory ' + 'responsible for making the function-maker-handler event. ' + 'Use this whenever\nyou need a function-maker-handler event' + ' made for you programmatically.'), + ArgInfo(name='function_maker_handler_event_factory2', + description='The function-maker-handler event factory ' + 'responsible for making the function-maker-handler event. ' + 'Use this whenever\nyou need a function-maker-handler event' + ' made for you programmatically.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + + def test_google_format_long_args_short_description(self): + docstring = """This is a Google-style docstring with long args. + + Args: + param1_that_is_very_longer_than_usual: The first 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 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.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + + def test_google_format_multiple_long_args_short_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 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 two lines.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + + def test_google_format_multiple_long_args_mixed_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. + 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.'), + 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. From 446065d75b16f8f21772f710e2c4e83a69b2ca07 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Sat, 13 Jan 2024 18:02:36 -0600 Subject: [PATCH 08/11] Updated code to per-arg state --- fire/docstrings.py | 63 ++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 7bb11ed2..b385bec9 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -177,12 +177,8 @@ def parse(docstring): state.returns.lines = [] state.yields.lines = [] state.raises.lines = [] - state.line1 = None - state.line1_length = None - state.line2_first_word_length = None - state.line2_length = None - state.line3_first_word_length = None - state.max_line_length = 0 + state.max_line_length = max(len(line) for line in lines) + for index, line in enumerate(lines): has_next = index + 1 < lines_len @@ -302,6 +298,11 @@ def _get_or_create_arg_by_name(state, name, is_kwarg=False): arg.name = name arg.type.lines = [] arg.description.lines = [] + arg.line1 = None + arg.line1_length = None + arg.line2_first_word_length = None + arg.line2_length = None + arg.line3_first_word_length = None if is_kwarg: state.kwargs.append(arg) else: @@ -397,13 +398,14 @@ 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: - state.line1 = line_info.line - state.line1_length = len(line_info.line) - state.max_line_length = max(state.max_line_length, state.line1_length) + + first, second = split_line # first is either the "arg" or "arg (type)" if _is_arg_name(first.strip()): arg = _get_or_create_arg_by_name(state, first.strip()) arg.description.lines.append(second.strip()) + arg.line1 = line_info.line + arg.line1_length = len(line_info.line) state.current_arg = arg else: arg_name_and_type = _as_arg_name_and_type(first) @@ -412,27 +414,31 @@ def _consume_google_args_line(line_info, state): arg = _get_or_create_arg_by_name(state, arg_name) arg.type.lines.append(type_str) arg.description.lines.append(second.strip()) + arg.line1 = line_info.line + arg.line1_length = len(line_info.line) state.current_arg = arg else: if state.current_arg: state.current_arg.description.lines.append(split_line[0]) + else: if state.current_arg: state.current_arg.description.lines.append(split_line[0]) - state.max_line_length = max(state.max_line_length, len(line_info.line)) - if line_info.previous.line == state.line1: # check for line2 + + if line_info.previous.line == state.current_arg.line1: # check for line2 line2_first_word = line_info.line.strip().split(' ')[0] - state.line2_first_word_length = len(line2_first_word) - state.line2_length = len(line_info.line) + 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.line3_first_word_length = len(line3_first_word) + state.current_arg.line3_first_word_length = len(line3_first_word) else: - state.line3_first_word_length = None + state.current_arg.line3_first_word_length = None else: - state.line2_first_word_length = state.line2_length = None + state.current_arg.line2_first_word_length = None + state.current_arg.line2_length = None def _merge_if_long_arg(state): @@ -442,24 +448,27 @@ def _merge_if_long_arg(state): state: The state of the docstring parser. """ actual_max_line_len = roundup(state.max_line_length) - arg_length = len(state.current_arg.name) + print(state.max_line_length, actual_max_line_len) + arg = state.current_arg + arg_length = len(arg.name) percent_105 = 1.05 * actual_max_line_len - long_arg_name = roundup(arg_length, 5) >= 0.4 * actual_max_line_len + long_arg_name = roundup(arg_length) >= 0.4 * actual_max_line_len + print(arg_length, long_arg_name) if long_arg_name: - if state.line2_first_word_length: - line1_plus_first_word = state.line1_length + state.line2_first_word_length + if arg.line2_first_word_length: + line1_plus_first_word = arg.line1_length + arg.line2_first_word_length line1_intentionally_short = roundup(line1_plus_first_word) < actual_max_line_len - line1_intentionally_long = state.line1_length >= percent_105 - line2_intentionally_long = state.line2_length >= percent_105 - if state.line3_first_word_length: - line2_plus_first_word = state.line2_length + state.line3_first_word_length + line1_intentionally_long = arg.line1_length >= percent_105 + line2_intentionally_long = arg.line2_length >= percent_105 + if arg.line3_first_word_length: + line2_plus_first_word = arg.line2_length + arg.line3_first_word_length line2_intentionally_short = roundup(line2_plus_first_word) < actual_max_line_len if not line1_intentionally_short and not line1_intentionally_long: if not line2_intentionally_short and not line2_intentionally_long: - _merge_line1_line2(state.current_arg.description.lines) + _merge_line1_line2(arg.description.lines) elif not line1_intentionally_short and not line1_intentionally_long: if not line2_intentionally_long: - _merge_line1_line2(state.current_arg.description.lines) + _merge_line1_line2(arg.description.lines) def _merge_line1_line2(lines): @@ -551,7 +560,7 @@ def _consume_line(line_info, state): if state.section.title == Sections.ARGS: if state.section.format == Formats.GOOGLE: _consume_google_args_line(line_info, state) - if state.current_arg and line_info.previous.line == state.line1: + if state.current_arg and line_info.previous.line == state.current_arg.line1: _merge_if_long_arg(state) elif state.section.format == Formats.RST: state.current_arg.description.lines.append(line_info.remaining.strip()) From 2458562690a1bff030cfc1cd191c40b79855d11c Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Sat, 13 Jan 2024 18:03:25 -0600 Subject: [PATCH 09/11] Updated to per-arg state --- fire/docstrings.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index b385bec9..dc2a33ce 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -448,12 +448,10 @@ def _merge_if_long_arg(state): state: The state of the docstring parser. """ actual_max_line_len = roundup(state.max_line_length) - print(state.max_line_length, actual_max_line_len) arg = state.current_arg arg_length = len(arg.name) percent_105 = 1.05 * actual_max_line_len long_arg_name = roundup(arg_length) >= 0.4 * actual_max_line_len - print(arg_length, long_arg_name) if long_arg_name: if arg.line2_first_word_length: line1_plus_first_word = arg.line1_length + arg.line2_first_word_length From 5ce77266a3178b19147e914ac16d82a3791d8bc6 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Sat, 13 Jan 2024 19:53:13 -0600 Subject: [PATCH 10/11] made code more robust and added test cases (handling :) --- fire/docstrings.py | 51 ++++++++++++++++---------- fire/docstrings_test.py | 80 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 19 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index dc2a33ce..926195c0 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -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 @@ -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) 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 @@ -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 @@ -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): diff --git a/fire/docstrings_test.py b/fire/docstrings_test.py index cba26ab7..6068b08e 100644 --- a/fire/docstrings_test.py +++ b/fire/docstrings_test.py @@ -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. From 794f48ed758bcff174f03b04604f22cb556aad36 Mon Sep 17 00:00:00 2001 From: Eminem <58310848+thebadcoder96@users.noreply.github.com> Date: Sat, 13 Jan 2024 20:57:17 -0600 Subject: [PATCH 11/11] Updated sttring comparison to index --- fire/docstrings.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 926195c0..a3f5a118 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -183,7 +183,7 @@ def parse(docstring): has_next = index + 1 < lines_len previous_line = lines[index - 1] if index > 0 else None next_line = lines[index + 1] if has_next else None - line_info = _create_line_info(line, next_line, previous_line) + line_info = _create_line_info(line, next_line, previous_line, index) _consume_line(line_info, state) summary = ' '.join(state.summary.lines) if state.summary.lines else None @@ -297,7 +297,7 @@ def _get_or_create_arg_by_name(state, name, is_kwarg=False): arg.name = name arg.type.lines = [] arg.description.lines = [] - arg.line1 = None + arg.line1_index = None arg.line1_length = None arg.line2_first_word_length = None arg.line2_length = None @@ -402,7 +402,7 @@ def _consume_google_args_line(line_info, state): 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 + arg.line1_index = line_info.index arg.line1_length = len(line_info.line) state.current_arg = arg else: @@ -412,14 +412,13 @@ def _consume_google_args_line(line_info, state): arg = _get_or_create_arg_by_name(state, arg_name) arg.type.lines.append(type_str) arg.description.lines.append(second.strip()) - arg.line1 = line_info.line + arg.line1_index = line_info.index arg.line1_length = len(line_info.line) state.current_arg = arg else: if state.current_arg: 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]) @@ -433,10 +432,10 @@ def _check_line2_line3(line_info, state): 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 + if line_info.previous.index == state.current_arg.line1_index: # line2 check 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) + 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: @@ -468,12 +467,14 @@ def _merge_if_long_arg(state): if long_arg_name: if arg.line2_first_word_length: line1_plus_first_word = arg.line1_length + arg.line2_first_word_length - line1_intentionally_short = roundup(line1_plus_first_word) < actual_max_line_len + line1_plus_first_word = roundup(line1_plus_first_word) + line1_intentionally_short = line1_plus_first_word < actual_max_line_len line1_intentionally_long = arg.line1_length >= percent_105 line2_intentionally_long = arg.line2_length >= percent_105 if arg.line3_first_word_length: line2_plus_first_word = arg.line2_length + arg.line3_first_word_length - line2_intentionally_short = roundup(line2_plus_first_word) < actual_max_line_len + line2_plus_first_word = roundup(line2_plus_first_word) + line2_intentionally_short = line2_plus_first_word < actual_max_line_len if not line1_intentionally_short and not line1_intentionally_long: if not line2_intentionally_short and not line2_intentionally_long: _merge_line1_line2(arg.description.lines) @@ -571,8 +572,9 @@ def _consume_line(line_info, state): if state.section.title == Sections.ARGS: if state.section.format == Formats.GOOGLE: _consume_google_args_line(line_info, state) - if state.current_arg and line_info.previous.line == state.current_arg.line1: - _merge_if_long_arg(state) + if state.current_arg: + if line_info.previous.index == state.current_arg.line1_index: + _merge_if_long_arg(state) elif state.section.format == Formats.RST: state.current_arg.description.lines.append(line_info.remaining.strip()) elif state.section.format == Formats.NUMPY: @@ -619,9 +621,10 @@ def _consume_line(line_info, state): pass -def _create_line_info(line, next_line, previous_line): +def _create_line_info(line, next_line, previous_line, index): """Returns information about the current line and surrounding lines.""" line_info = Namespace() # TODO(dbieber): Switch to an explicit class. + line_info.index = index line_info.line = line line_info.stripped = line.strip() line_info.remaining_raw = line_info.line @@ -631,10 +634,12 @@ def _create_line_info(line, next_line, previous_line): line_info.next.line = next_line next_line_exists = next_line is not None line_info.next.stripped = next_line.strip() if next_line_exists else None + line_info.next.index = index + 1 if next_line_exists else None line_info.next.indentation = ( len(next_line) - len(next_line.lstrip()) if next_line_exists else None) line_info.previous.line = previous_line previous_line_exists = previous_line is not None + line_info.previous.index = index - 1 if previous_line_exists else None line_info.previous.indentation = ( len(previous_line) - len(previous_line.lstrip()) if previous_line_exists else None)