[pydoclint] Permit yielding None in DOC402 and DOC403

[tjkuson]

Summary

Currently, docstring-extraneous-yields (DOC403) reports a diagnostic if there is a yields section in a docstring for a function that yields None implicitly; for example,

from collections import abc

def gen() -> abc.Generator[None, None, None]:
    """A very helpful docstring.

    Yields:
        When foo.
    """
    ...  # some code
    yield
    ...  # some more code

This is problematic, as sometimes you want to document this yielding behaviour.

This patch permits such docstrings by now counting implicit yield None statements as yields. It then updates docstring-missing-yields (DOC402) to allow users to forego a yield section if the function is resolved to yield None only (by checking the return annotation, or by checking the yield statements if the return type is not annotated).

This is similar to #13064

Closes #13001

Test Plan

cargo nextest run

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+6 -41 violations, +0 -0 fixes in 3 projects; 51 projects unchanged)

apache/airflow (+5 -7 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ airflow/cli/cli_config.py:140:9: DOC201 `return` is not documented in docstring
- airflow/cli/cli_config.py:141:5: DOC201 `return` is not documented in docstring
+ airflow/metrics/otel_logger.py:198:13: DOC201 `return` is not documented in docstring
- airflow/metrics/otel_logger.py:205:13: DOC201 `return` is not documented in docstring
+ airflow/metrics/otel_logger.py:224:13: DOC201 `return` is not documented in docstring
- airflow/metrics/otel_logger.py:231:13: DOC201 `return` is not documented in docstring
+ airflow/providers/airbyte/sensors/airbyte.py:127:17: DOC201 `return` is not documented in docstring
... 1 additional changes omitted for rule DOC201
- airflow/providers/amazon/aws/hooks/logs.py:142:13: DOC402 `yield` is not documented in docstring
- airflow/providers/amazon/aws/hooks/sagemaker.py:276:13: DOC402 `yield` is not documented in docstring
- airflow/providers/grpc/hooks/grpc.py:137:21: DOC402 `yield` is not documented in docstring
+ airflow/triggers/base.py:102:9: DOC402 `yield` is not documented in docstring
- tests/test_utils/asserts.py:159:9: DOC402 `yield` is not documented in docstring

apache/superset (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ superset/utils/core.py:1305:13: DOC402 `yield` is not documented in docstring

pandas-dev/pandas (+0 -34 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- doc/source/user_guide/style.ipynb:cell 113:31:89: E501 Line too long (90 > 88)
- doc/source/user_guide/style.ipynb:cell 113:33:89: E501 Line too long (98 > 88)
- doc/source/user_guide/style.ipynb:cell 123:5:56: E741 Ambiguous variable name: `l`
- doc/source/user_guide/style.ipynb:cell 125:2:13: C408 Unnecessary `dict` call (rewrite as a literal)
- doc/source/user_guide/style.ipynb:cell 125:4:13: C408 Unnecessary `dict` call (rewrite as a literal)
- doc/source/user_guide/style.ipynb:cell 125:6:13: C408 Unnecessary `dict` call (rewrite as a literal)
- doc/source/user_guide/style.ipynb:cell 125:8:13: C408 Unnecessary `dict` call (rewrite as a literal)
- doc/source/user_guide/style.ipynb:cell 126:1:1: NPY002 Replace legacy `np.random.seed` call with `np.random.Generator`
- doc/source/user_guide/style.ipynb:cell 126:2:1: PLW0127 Self-assignment of variable `cmap`
- doc/source/user_guide/style.ipynb:cell 126:2:8: PLW0128 Redeclared variable `cmap` in assignment
- doc/source/user_guide/style.ipynb:cell 126:3:22: NPY002 Replace legacy `np.random.randn` call with `np.random.Generator`
- doc/source/user_guide/style.ipynb:cell 128:1:22: NPY002 Replace legacy `np.random.randn` call with `np.random.Generator`
- doc/source/user_guide/style.ipynb:cell 134:1:89: E501 Line too long (93 > 88)
- doc/source/user_guide/style.ipynb:cell 141:1:89: E501 Line too long (95 > 88)
- doc/source/user_guide/style.ipynb:cell 157:1:38: F811 Redefinition of unused `f` from cell 123, line 5
- doc/source/user_guide/style.ipynb:cell 15:2:89: E501 Line too long (103 > 88)
- doc/source/user_guide/style.ipynb:cell 15:3:89: E501 Line too long (156 > 88)
... 13 additional changes omitted for rule E501
- doc/source/user_guide/style.ipynb:cell 37:1:1: NPY002 Replace legacy `np.random.seed` call with `np.random.Generator`
- doc/source/user_guide/style.ipynb:cell 37:2:20: NPY002 Replace legacy `np.random.randn` call with `np.random.Generator`
- doc/source/user_guide/style.ipynb:cell 60:1:20: NPY002 Replace legacy `np.random.randn` call with `np.random.Generator`
... 3 additional changes omitted for rule NPY002

Changes by rule (9 rules affected)

code	total	+ violation	- violation	+ fix	- fix
E501	18	0	18	0	0
NPY002	8	0	8	0	0
DOC201	7	4	3	0	0
DOC402	6	2	4	0	0
C408	4	0	4	0	0
E741	1	0	1	0	0
PLW0127	1	0	1	0	0
PLW0128	1	0	1	0	0
F811	1	0	1	0	0

[AlexWaygood]

So, consider this ecosystem hit: https://github.com/apache/superset/blob/07985e2f5aa165f6868abbf88594e6d75300caae/superset/utils/core.py#L1288-L1312. The yield is "documented" there in that it's clear that the function is a context manager that only temporarily overrides a value. But there's no "Yields" section -- and I think it would make the docstring worse if they rewrote it to include a "Yields" section; but this change would mean that we'd emit DOC402 on that docstring.

I think my preferred behaviour here would be:

• Don't report DOC402 for a function that only ever has bare yield or yield None statements
• But also, don't report DOC403 just because the yield is documented

I think this is what you suggested in #13001 (comment) (sorry that nobody responded to you there!). It's a little different from numpydoc's behaviour IIUC, but I think that's okay.

[AlexWaygood]

(Oh and thanks for the PR! Sorry for diving straight into the review 😄)

[tjkuson]

So, consider this ecosystem hit: https://github.com/apache/superset/blob/07985e2f5aa165f6868abbf88594e6d75300caae/superset/utils/core.py#L1288-L1312.

Would you mind clarifying this and how it interacts with the current implementation of DOC201? From what I understand, its annotated yield type is typing.Any, so its yield behaviour should be documented (it's not None).

If the recommendation is to ignore the type annotation if we determine all yields to be None by parsing the function body, this would be inconsistent with DOC201 behaviour of treating the return annotation as truth #13064. Would that implementation have to change as well, or would it just be a documented discrepancy?

[AlexWaygood]

So, consider this ecosystem hit: apache/superset@07985e2/superset/utils/core.py#L1288-L1312.

Would you mind clarifying this and how it interacts with the current implementation of DOC201? From what I understand, its annotated yield type is typing.Any, so its yield behaviour should be documented (it's not None).

Ahh, thanks for pointing that out. Your argument makes sense; I agree that it's important for these rules to remain consistent.

Your PR however creates a new inconsistency between the two rules as it currently stands, however. DOC201 will currently complain about the first of these two docstrings, but not the second:

def foo() -> int | None:
    """Foo-y method"""
    return None


def bar() -> int | None:
    """Bar-y method"""
    return

Would you like to fix that as well as part of this PR, so that the rules stay consistent?

[tjkuson]

Would you like to fix that as well as part of this PR, so that the rules stay consistent?

Fixed in 79fad1c!

[codspeed-hq]

CodSpeed Performance Report

Merging #13148 will not alter performance

_{Comparing tjkuson:doc-yield-none (117fdcc) with main (0c23b86)}

Summary

✅ 32 untouched benchmarks

[charliermarsh]

This makes sense to me, but I'll defer to @AlexWaygood to approve and merge.

[AlexWaygood]

Thanks @tjkuson!