DOC403 behaviour with yielding none

[jonyscathe]

• List of keywords you searched for before creating this issue. Write them down here so that others can find this issue more easily and help provide feedback.
  "DOC403 ", "numpydoc", "yields"
• A minimal code snippet that reproduces the bug.

@pytest.fixture
def db_setup() -> Generator[None, None, None]:
    """
    Add test user.

    Yields
    ------
        And then cleans up test user.
    """
    database = Database('my_database_url')
    with database.create_session() as session:
        session.execute(insert(database.Users, [{'id': 1, 'name': 'Bob'}

    yield

    with database.create_session() as session:
        session.execute(delete(database.Users).where(database.Users.id == 1234)

• The command you invoked (e.g., ruff /path/to/file.py --fix), ideally including the --isolated flag.
  ruff /path/to/file.py --fix
• The current Ruff settings (any relevant sections from your pyproject.toml).

[tool.ruff]
line-length = 120
preview = true
target-version = 'py312'

[tool.ruff.format]
line-ending = 'lf'
quote-style = 'single'

[tool.ruff.lint]
extend-select = ['D213']
ignore = ['ANN401', 'CPY001', 'D107', 'D203', 'D212', 'E203', 'ISC003', 'PLC1901', 'PLR6104']
select = ['ALL']

[tool.ruff.lint.flake8-implicit-str-concat]
allow-multiline = false

[tool.ruff.lint.flake8-quotes]
inline-quotes = 'single'

[tool.ruff.lint.flake8-type-checking]
strict = true

[tool.ruff.lint.pydocstyle]
convention = 'numpy'

[tool.ruff.lint.pylint]
max-args = 14
max-statements = 75

• The current Ruff version (ruff --version).
  ruff 0.6.1

The above code snippet results in a DOC403 Docstring has a "Yields" section but the function doesn't yield anything.

If the yields section is removed from the docstring then numpydoc-validation reports:

+---------------------+-------------------+---------+-------------------------+
| file                | item              | check   | description             |
+=====================+===================+=========+=========================+
| src/conftest.py:135 | conftest.db_setup | YD01    | No Yields section found |
+---------------------+-------------------+---------+-------------------------+

I understand that it is debatable about what the docstring should be a for a yield statement that yields None, but I think ruff should probably match the numpydoc-validator tool in this case.

[dhruvmanila]

This seems like a bug. An empty yield isn't considered to be present in the body:

ruff/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs

Lines 638 to 644 in 4d0d3b0

	match expr {
	Expr::Yield(ast::ExprYield {
	range,
	value: Some(_),
	}) => {
	self.yields.push(Entry { range: *range });
	}

@augustelalande just checking in to see if there's a specific reason behind this otherwise we can remove the Some(_) condition from above.

[augustelalande]

I did this on purpose because IMO the yields section should document the yielded value not, the function behaviour associated with the yield. Similar to return None, not needing documentation.

The yield "behaviour" could be documented in the docstring summary.

But if people feel strongly the other way, feel free to change the behaviour.

[jonyscathe]

@augustelalande to be fair I do agree with your logic. I only raised this due to conflicting behaviour with numpydoc's validation tool.
Maybe I will raise an issue with numpydoc and see what they say about it.

[dhruvmanila]

I did this on purpose because IMO the yields section should document the yielded value not, the function behaviour associated with the yield. Similar to return None, not needing documentation.

Yeah, that makes sense.

Maybe I will raise an issue with numpydoc and see what they say about it.

@jonyscathe That might be useful, thanks

[jonyscathe]

The numpydoc people do believe that the yield should be documented even if it is none, as it is not really the same thing as returning none (the yield is still doing something).
numpy/numpydoc#580

They also have the example that something like:

def yielder():
    for _ in range(10):
        yield

is inherently different to:

def yielder():
    yield

And should be documented as such in the Yields section (10 implicit None's via a generator vs 1).

[larsoner]

... and just to bring the discussion over here to save some a click, I suggested:

It seems reasonable that [these two examples] would both need some Yields section of some sort to tell people about the nature of the returned generator.

It seems natural to put this in a Yields section. But I am not 100% convinced either way. cc @stefanv who was also discussing over in numpydoc.

[dhruvmanila]

@jonyscathe @larsoner Thanks for confirming! I think it makes sense to update the behavior. All we need is to remove the Some(_) match as mentioned in #13001 (comment).

It seems that pydoclint doesn't raise an issue for the source code mentioned in the PR description.

[tjkuson]

Is the suggestion to report docstring-missing-yields (DOC402) on

@pytest.fixture
def db_setup() -> Generator[None, None, None]:
    """
    Add test user.

    """
    database = Database('my_database_url')
    with database.create_session() as session:
        session.execute(insert(database.Users, [{'id': 1, 'name': 'Bob'}

    yield

    with database.create_session() as session:
        session.execute(delete(database.Users).where(database.Users.id == 1234)

as well?

If it is unclear whether implicit yield None requires a yields section in the docstring, it makes sense to not report a DOC403 upon its absence. However, it might also make sense to not report DOC402 if an implicit yield None is present without an accompanying docstring section — i.e., to permit the above code example. In many situations, an implicit yield None is trivial and only occurs once, and thus a yields section would only exist to document that the object is a generator (already achieved via a typing.Generator annotation).

If we want both DOC403 and DOC402 make exceptions for implicit yields (that is to say, count them in one and ignore in them in the other), then it is more than a one-line code change.

Possibly related to #13062