Skip to content

issue-261 Fixed missing parts of argument descriptions#262

Closed
MichaelCG8 wants to merge 1 commit intogoogle:masterfrom
MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions
Closed

issue-261 Fixed missing parts of argument descriptions#262
MichaelCG8 wants to merge 1 commit intogoogle:masterfrom
MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions

Conversation

@MichaelCG8
Copy link
Copy Markdown
Contributor

@MichaelCG8 MichaelCG8 commented May 23, 2020

issue-261 Fixed missing parts of argument descriptions for Google and Numpy style docstrings.

@googlebot googlebot added the cla: yes Author has signed CLA label May 23, 2020
@MichaelCG8 MichaelCG8 mentioned this pull request May 23, 2020
Closed
@dbieber
Copy link
Copy Markdown
Collaborator

dbieber commented Dec 16, 2020 •
edited
Loading

Belated thanks for this PR!

If I'm understanding correctly:

There are two behavior changes within:

  1. We now use arg_pattern = r'^[a-zA-Z_]\w*$' for determining valid argument names.
  2. We now set state.current_arg = arg when we encounter a Google arg-and-type.

Change 1. doesn't seem so important -- it prevents the parser from treating some invalid arg names as args, but as I understand it this increased strictness hasn't come up in practice.

Change 2. is important; it fixes a bug where previously we were losing some information about multiline arg descriptions.

Can you confirm my understanding is correct?

Thanks.

Thanks also for the test cases 👍

@MichaelCG8
Copy link
Copy Markdown
Contributor Author

MichaelCG8 commented Jan 17, 2021

With similar belatedness, thank you for reviewing!

Change 1
The old code would treat a line as an arg name if it is one word long and doesn't contain a :. The case that fails is any multiline description where the last line consists of one word only, as in foo3 #261 (comment)

This could have been fixed by also checking for a . in the string, so that any text ended with a period would be parsed as the plaintext description. The regex is more general and fixes the behaviour for any string that isn't a valid argument pattern, for example if the docstring ends with a reference to a type by surrounding it in back ticks.

The case where there is a single word followed by a period is something I've hit a few times if I reach my line length limit and wrap the last word onto a new line.

Change 2
Spot on.

python-fire-bot pushed a commit that referenced this pull request Jan 22, 2021
@MichaelCG8 @copybara-github
issue-261 Fixed missing parts of argument descriptions for Google and…
c39de6a 
… Numpy style docstrings.

482bb9d by Michael Garbutt <[email protected]>:

COPYBARA_INTEGRATE_REVIEW=#262 from MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions 482bb9d
PiperOrigin-RevId: 353248737
Change-Id: I3033200c69c5aba0ce9bb819574f37248e32c1cb
@dbieber
Copy link
Copy Markdown
Collaborator

dbieber commented Jan 22, 2021

Closed with c39de6a

@dbieber dbieber closed this Jan 22, 2021
CherylMoonba added a commit to CherylMoonba/devices that referenced this pull request Jul 20, 2025
@CherylMoonba
issue-261 Fixed missing parts of argument descriptions for Google and…
0ba8507 
… Numpy style docstrings.

482bb9da4bc09522984d982716a79caeafabf3e2 by Michael Garbutt <[email protected]>:

COPYBARA_INTEGRATE_REVIEW=google/python-fire#262 from MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions 482bb9da4bc09522984d982716a79caeafabf3e2
PiperOrigin-RevId: 353248737
Change-Id: I3033200c69c5aba0ce9bb819574f37248e32c1cb
KurtMuelle added a commit to KurtMuelle/obtained that referenced this pull request Jul 22, 2025
@KurtMuelle
issue-261 Fixed missing parts of argument descriptions for Google and…
fdaba23 
… Numpy style docstrings.

482bb9da4bc09522984d982716a79caeafabf3e2 by Michael Garbutt <[email protected]>:

COPYBARA_INTEGRATE_REVIEW=google/python-fire#262 from MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions 482bb9da4bc09522984d982716a79caeafabf3e2
PiperOrigin-RevId: 353248737
Change-Id: I3033200c69c5aba0ce9bb819574f37248e32c1cb
hwlegends pushed a commit to hwlegends/python-fire that referenced this pull request Aug 20, 2025
@MichaelCG8 @copybara-github
issue-261 Fixed missing parts of argument descriptions for Google and…
0a33527 
… Numpy style docstrings.

482bb9da4bc09522984d982716a79caeafabf3e2 by Michael Garbutt <[email protected]>:

COPYBARA_INTEGRATE_REVIEW=google/python-fire#262 from MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions 482bb9da4bc09522984d982716a79caeafabf3e2
PiperOrigin-RevId: 353248737
Change-Id: I3033200c69c5aba0ce9bb819574f37248e32c1cb
Hernandez19891 added a commit to Hernandez19891/Illumina that referenced this pull request Sep 15, 2025
@Hernandez19891
issue-261 Fixed missing parts of argument descriptions for Google and…
317c296 
… Numpy style docstrings.

482bb9da4bc09522984d982716a79caeafabf3e2 by Michael Garbutt <[email protected]>:

COPYBARA_INTEGRATE_REVIEW=google/python-fire#262 from MichaelCG8:issue-261-bugfix-multi-line-docstring-parameter-descriptions 482bb9da4bc09522984d982716a79caeafabf3e2
PiperOrigin-RevId: 353248737
Change-Id: I3033200c69c5aba0ce9bb819574f37248e32c1cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cla: yes Author has signed CLA

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MichaelCG8 @dbieber @googlebot