bug: Google docstrings: no support for non-multiple or non-named values in Yields section

the-13th-letter commented 5 months ago

Description of the bug

In Google-style docstrings, Griffe (and mkdocstrings-python) don't treat Yields and Returns sections identically when it comes to returning multiple and/or named items. In particular, for Returns sections, the returns_multiple_items and returns_named_values configuration options exist and do as advertised. For Yields sections, no such dedicated options exist, and those options are ignored. As a consequence, for multiple values, there is no way to document the yielded tuple of values as a tuple; it must be faked via a single item tuple, the type repeated (actually: overruled) in the docstring, and the description must be written with hanging indentation.

Note: The Google style guide, at least as of version 8487c08, treats "Yields:" and "Returns:" identically and requires them to document a tuple of returned values as a tuple, explicitly forbidding the numpy-style named tuple return values.

To Reproduce

Given the following example module yielding_test

"""Test "Yields:" sections in Google-style docstrings."""

from __future__ import annotations

from collections.abc import Iterator

def return_one() -> str:
    """XXX

    Returns:
        Returns one item.  Returns one item.  Returns one item.  Returns one
        item.  Returns one item.  Returns one item.

    """
    return "one"

def return_two() -> tuple[str, int]:
    """XXX

    Returns:
        retval: Returns two items.  Returns two items.  Returns two items.
        Returns two items.  Returns two items.  Returns two items.

    """
    return "one", 2

def yield_one() -> Iterator[str]:
    """XXX

    Yields:
        Yields one item.  Yields one item.  Yields one item.  Yields one
        item.  Yields one item.  Yields one item.

    """
    yield "one"

def yield_two() -> Iterator[tuple[str, int]]:
    """XXX

    Yields:
        yieldval: Yields two items.  Yields two items.  Yields two
        items.  Yields two items.  Yields two items.  Yields two items.

    """
    yield "one", 2

and parsed with options returns_multiple_items and returns_named_value as false, the dump from Griffe shows that yield_one supposedly has two return values (reusing the type annotation for the "second" return value) and that yield_two also has two return values, the second one being unnamed.

Full traceback

Full Griffe dump

```console $ PYTHONPATH=. griffe dump -d google -D '{"returns_multiple_items": false, "returns_named_value": false}' -f -LDEBUG yielding_test INFO Loading package yielding_test DEBUG Found yielding_test: loading DEBUG Loading path /tmp/XYZ/yielding_test.py INFO Finished loading packages { "yielding_test": { "kind": "module", "name": "yielding_test", "path": "yielding_test", "filepath": "/tmp/XYZ/yielding_test.py", "relative_filepath": "yielding_test.py", "relative_package_filepath": "XYZ/yielding_test.py", "docstring": { "value": "Test \"Yields:\" sections in Google-style docstrings.", "lineno": 1, "endlineno": 1, "parsed": [ { "kind": "text", "value": "Test \"Yields:\" sections in Google-style docstrings." } ] }, "labels": [], "members": [ { "kind": "alias", "name": "annotations", "target_path": "__future__.annotations", "path": "yielding_test.annotations", "lineno": 3, "endlineno": 3 }, { "kind": "alias", "name": "Iterator", "target_path": "collections.abc.Iterator", "path": "yielding_test.Iterator", "lineno": 5, "endlineno": 5 }, { "kind": "function", "name": "return_one", "path": "yielding_test.return_one", "filepath": "/tmp/XYZ/yielding_test.py", "relative_filepath": "yielding_test.py", "relative_package_filepath": "XYZ/yielding_test.py", "lineno": 7, "endlineno": 15, "docstring": { "value": "XXX\n\nReturns:\n Returns one item. Returns one item. Returns one item. Returns one\n item. Returns one item. Returns one item.", "lineno": 8, "endlineno": 14, "parsed": [ { "kind": "text", "value": "XXX" }, { "kind": "returns", "value": [ { "name": "", "annotation": { "name": "str", "cls": "ExprName" }, "description": "Returns one item. Returns one item. Returns one item. Returns one\nitem. Returns one item. Returns one item." } ] } ] }, "labels": [], "members": [], "decorators": [], "parameters": [], "returns": { "name": "str", "cls": "ExprName" } }, { "kind": "function", "name": "return_two", "path": "yielding_test.return_two", "filepath": "/tmp/XYZ/yielding_test.py", "relative_filepath": "yielding_test.py", "relative_package_filepath": "XYZ/yielding_test.py", "lineno": 17, "endlineno": 25, "docstring": { "value": "XXX\n\nReturns:\n retval: Returns two items. Returns two items. Returns two items.\n Returns two items. Returns two items. Returns two items.", "lineno": 18, "endlineno": 24, "parsed": [ { "kind": "text", "value": "XXX" }, { "kind": "returns", "value": [ { "name": "", "annotation": { "name": "retval", "cls": "ExprName" }, "description": "Returns two items. Returns two items. Returns two items.\nReturns two items. Returns two items. Returns two items." } ] } ] }, "labels": [], "members": [], "decorators": [], "parameters": [], "returns": { "left": { "name": "tuple", "cls": "ExprName" }, "slice": { "elements": [ { "name": "str", "cls": "ExprName" }, { "name": "int", "cls": "ExprName" } ], "implicit": true, "cls": "ExprTuple" }, "cls": "ExprSubscript" } }, { "kind": "function", "name": "yield_one", "path": "yielding_test.yield_one", "filepath": "/tmp/XYZ/yielding_test.py", "relative_filepath": "yielding_test.py", "relative_package_filepath": "XYZ/yielding_test.py", "lineno": 27, "endlineno": 35, "docstring": { "value": "XXX\n\nYields:\n Yields one item. Yields one item. Yields one item. Yields one\n item. Yields one item. Yields one item.", "lineno": 28, "endlineno": 34, "parsed": [ { "kind": "text", "value": "XXX" }, { "kind": "yields", "value": [ { "name": "", "annotation": { "name": "str", "cls": "ExprName" }, "description": "Yields one item. Yields one item. Yields one item. Yields one" }, { "name": "", "annotation": { "name": "str", "cls": "ExprName" }, "description": "item. Yields one item. Yields one item." } ] } ] }, "labels": [], "members": [], "decorators": [], "parameters": [], "returns": { "left": { "name": "Iterator", "cls": "ExprName" }, "slice": { "name": "str", "cls": "ExprName" }, "cls": "ExprSubscript" } }, { "kind": "function", "name": "yield_two", "path": "yielding_test.yield_two", "filepath": "/tmp/XYZ/yielding_test.py", "relative_filepath": "yielding_test.py", "relative_package_filepath": "XYZ/yielding_test.py", "lineno": 37, "endlineno": 45, "docstring": { "value": "XXX\n\nYields:\n yieldval: Yields two items. Yields two items. Yields two\n items. Yields two items. Yields two items. Yields two items.", "lineno": 38, "endlineno": 44, "parsed": [ { "kind": "text", "value": "XXX" }, { "kind": "yields", "value": [ { "name": "yieldval", "annotation": { "name": "str", "cls": "ExprName" }, "description": "Yields two items. Yields two items. Yields two" }, { "name": "", "annotation": { "name": "int", "cls": "ExprName" }, "description": "items. Yields two items. Yields two items. Yields two items." } ] } ] }, "labels": [], "members": [], "decorators": [], "parameters": [], "returns": { "left": { "name": "Iterator", "cls": "ExprName" }, "slice": { "left": { "name": "tuple", "cls": "ExprName" }, "slice": { "elements": [ { "name": "str", "cls": "ExprName" }, { "name": "int", "cls": "ExprName" } ], "implicit": true, "cls": "ExprTuple" }, "cls": "ExprSubscript" }, "cls": "ExprSubscript" } } ] } } ``` Note that the `docstring.parsed[1].value` arrays contain one item for the `returns_*` functions, and two items for the `yields_*` functions. The latter is unexpected.

Expected behavior

On a high level, I expect the output for the yields_one and yields_two to be similar to the respective return_one and return_two output (except for "kind": "returns" vs. "kind": "yields").

On a lower level, I expect the returns_multiple_items and returns_named_value options to also be respected when parsing Yields sections.

Environment information

Running Griffe v0.44.0 on Linux, installed via PyPI.

Additional context

Support for non-multiple return values and named return values seems to have originated in mkdocstrings/griffe#137 (0.35.0, 2023-08-24) and mkdocstrings/griffe#196 (0.36.0, 2023-09-01). In both cases only the Returns section handling was adapted, and Yields section handling was left untouched.

Additional comments

I do not advocate adding additional configuration options yields_multiple_items/yields_named_value or similar to handle this case. It makes sense to reuse the existing returns_* options, because Google style treats Returns and Yields so similarly.

I tried my hand at copying the respective code from src/griffe/docstrings/google.py:_read_returns_section to _read_yields_section, but I had trouble setting up new tests in tests/test_docstrings/test_google.py. I don't know enough about Griffe's internals (or pytest) to properly debug whether the copied code is bad or whether my tests are bad. (Or both.) So, no pull request, just a bug report. Sorry.

pawamoy commented 5 months ago

Hey again @the-13th-letter, thanks for the great report. I agree with the suggestion to make yields and returns consistent. Maybe also receives?

the-13th-letter commented 5 months ago

Hey again @the-13th-letter, thanks for the great report. I agree with the suggestion to make yields and returns consistent.

:)

Maybe also receives?

I thought about that as well, but I don't have a good case for this. Just a couple of disjointed random thoughts.

Google's style guide doesn't talk about generator input via generator.send(...) at all. So it's anyone's guess how to properly format this or even talk about this in a valid Google-style Python program.
The Receives block comes from numpydoc. So it's much more important that it is compatible with NumPy's style than with Google's style. numpydoc currently (v1.8.0rc0) says: “Since, like for Yields and Returns, a single object is always passed to the method, this may describe either the single parameter, or positional arguments passed as a tuple.” So there may be some desire for receives_multiple_items = False.
- The function analog to Receives is (positional-only) Args/Parameters. I cannot really imagine anyone seriously suggesting that the Args/Parameters block should support not accepting multiple parameters. Why would Receives be any different? So maybe there's no desire receives_multiple_items = False.
I find the need for receives_named_value = False somewhat unlikely: you generally do spam, eggs, *parrots = yield ... inside the generator, so you already have “parameter names” (spam, eggs, *parrots) that you probably want to use in the Receives block.
- But then, perhaps it's a spam = yield ... case where spam is an internal name that you don't want to expose, and now the docs are forcing you to name things anyway...

So yeah, I don't really know what to do with Receives. And I don't think I'm the right person to ask about that either, because I use neither Receives nor the NumPy docstring format in general.

pawamoy commented 5 months ago

Thanks for your thoughts.

So yeah, I don't really know what to do with Receives. And I don't think I'm the right person to ask about that either, because I use neither Receives nor the NumPy docstring format in general.

Then I'd vote we make "Receives" consistent with the rest, for the sake of consistency. Both Google-style and Numpydoc-style are underspecified, so I always allowed myself some wiggle-room with the goal of making both styles more similar, structurally speaking.

mkdocstrings / griffe