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

Extra table rows for Returns when empty lines start with spaces #129

Closed
LukeSavefrogs opened this issue Feb 14, 2023 · 5 comments
Closed

Comments

@LukeSavefrogs
Copy link

Describe the bug

When there are lines in the docstring that are not completely empty (but have spaces or tabs in it) between the end of the Returns block and the next block (in my case Example) extra table rows are added.

To Reproduce

Steps to reproduce the behavior:

  1. Write the following in a docstring:
    def test():
        """Test docstring with 2 extra empty parameters.
    
        Returns:
            str: TEST
            
            
        Example:
            ```pycon
            >>> test()
            ```
        """
  2. A similar output should be shown:
    image

This however does not happen if there are no spaces (only empty lines) between the last parameter and the next block (in this case Example)

def test():
    """Test docstring with 2 extra empty parameters.

    Returns:
        str: TEST


    Example:
        ```pycon
        >>> test()
        ```
    """

image

System information

  • griffe version: 0.25.4
  • Python version: 3.9
  • OS: Windows
@LukeSavefrogs
Copy link
Author

LukeSavefrogs commented Feb 14, 2023

Found the "culprit"...

By placing a print(locals()) right before the return of the Google docstring handler i get this (i reformatted it to json, so True became true, None became null and so on...) i got this:

{
    "docstring": "<griffe.dataclasses.Docstring object at 0x00000219C5854AC0>",
    "ignore_init_summary": false,
    "trim_doctest_flags": true,
    "options": {
        "ignore_init_summary": false,
        "trim_doctest_flags": true
    },
    "sections": [
        "<griffe.docstrings.dataclasses.DocstringSectionText object at 0x00000219C5873160>",
        "<griffe.docstrings.dataclasses.DocstringSectionReturns object at 0x00000219C5880A60>",
        "<griffe.docstrings.dataclasses.DocstringSectionAdmonition object at 0x00000219C5880DC0>"
    ],
    "current_section": [],
    "in_code_block": false,
    "lines": [
        "Test docstring with 2 extra empty parameters.",
        "",
        "Returns:",
        "    str: TEST",
        "    ",
        "    ",
        "Example:",
        "    ```pycon",
        "    >>> test()",
        "    ```"
    ],
    "ignore_summary": false,
    "offset": 10,
    "line_lower": "example:",
    "match": "<re.Match object; span=(0, 8), match='Example: '>",
    "groups": {
        "type": "Example",
        "title": null
    },
    "title": "Example",
    "admonition_type": "example",
    "reader": "<function _read_returns_section at 0x00000219C5744E50>",
    "section": "<griffe.docstrings.dataclasses.DocstringSectionReturns object at 0x00000219C5880A60>",
    "contents": "```pycon\n>>> test()\n```"
}

As we can see the lines variable contains empty lines, which makes me think maybe the docstring object passed as a parameter is the one responsible...

I think the lines variable should instead be:

{
    [...]
    "lines": [
        "Test docstring with 2 extra empty parameters.",
        "",
        "Returns:",
        "    str: TEST",
        "",
        "Example:",
        "    ```pycon",
        "    >>> test()",
        "    ```"
    ]
    [...]
}

The docstring.lines property is defined here as self.value.split("\n"):

@cached_property
def lines(self) -> list[str]:
"""Returns the lines of the docstring.
Returns:
The docstring's lines.
"""
return self.value.split("\n")

Do you think that maybe we should normalize all empty lines by removing extra whitespace characters (regex: ^\s*$)?
This way the parsers could have a consistent value since i don't think any parser counts a line with spaces. In case this was not true, it could only be added to the Google docstring parser

UPDATE

I tried adding lines = list(map(lambda line: line.rstrip(), docstring.lines)) instead of lines = docstring.lines but the result is always the same even if the lines variable seems right..

lines = docstring.lines

@LukeSavefrogs
Copy link
Author

Solution (?)

I made some progress...

I found that the filtering on docstring.lines did not work because the function _read_returns_section (which is responsible for the creation of the returns section) read directly from the original docstring.

I solved temporarily by changing this line:

returns.append(DocstringReturn(name=name or "", annotation=annotation, description=description))

To:

if name is not None:
    returns.append(DocstringReturn(name=name or "", annotation=annotation, description=description))

This way the bug seems fixed:
image

Considerations

  • It would be better to move the check at the start, instead of right before the assignment (maybe something like if _is_empty_line(return_lines[0]): continue)
  • Should the other _read_*section functions be "patched" as well?

@pawamoy
Copy link
Member

pawamoy commented Feb 15, 2023

Hello, thanks for the report and investigation!

I've looked at it and the bug actually comes from the function that splits blocks of text (sections). If you look at this:

elif line.startswith(indent * " "):
# indent equal to initial one: new item
items.append(current_item)
current_item = (new_offset, [line[indent:]])
elif _is_empty_line(line):
# empty line: preserve it in the current item
current_item[1].append("")

...you might notice that the first condition does not check if the line is "empty" (just whitespace). So the solution would simply be to check that, and update the rest of the checks accordingly.

I should be able to push a fix tonight 🙂

@LukeSavefrogs
Copy link
Author

Thank you @pawamoy for the quick fix!

Do you know when will this fix be available in a release?

@pawamoy
Copy link
Member

pawamoy commented Feb 16, 2023

Just released 0.25.5 🙂

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