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

[pydoclint] Add docstring-missing-yields amd docstring-extraneous-yields (DOC402, DOC403) #12538

Merged
merged 9 commits into from
Aug 2, 2024

Conversation

augustelalande
Copy link
Contributor

Summary

Add docstring-missing-yields amd docstring-extraneous-yields (DOC402, DOC403). These rules check that the yields defined (or not) in the docstring of a function match its implementation.

The implementation is essentially a copy-paste of #12485

Part of #12434.

Test Plan

Test cases added.

@augustelalande
Copy link
Contributor Author

What happened to the ecosystem check? Why isn't it making a comment of the changes?

@charliermarsh
Copy link
Member

Not sure yet, it looks like the comment job can't find the artifact: https://github.com/astral-sh/ruff/actions/runs/10121063472/job/27991584964

Copy link
Contributor

github-actions bot commented Jul 27, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+205 -0 violations, +0 -0 fixes in 4 projects; 50 projects unchanged)

apache/airflow (+168 -0 violations, +0 -0 fixes)

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

+ airflow/api/common/mark_tasks.py:249:17: DOC402 `yield` is not documented in docstring
+ airflow/api/common/mark_tasks.py:288:13: DOC402 `yield` is not documented in docstring
+ airflow/cli/cli_parser.py:162:5: DOC402 `yield` is not documented in docstring
+ airflow/dag_processing/manager.py:842:21: DOC402 `yield` is not documented in docstring
+ airflow/datasets/__init__.py:224:9: DOC402 `yield` is not documented in docstring
+ airflow/datasets/__init__.py:304:9: DOC402 `yield` is not documented in docstring
+ airflow/datasets/__init__.py:344:13: DOC402 `yield` is not documented in docstring
+ airflow/datasets/__init__.py:403:17: DOC402 `yield` is not documented in docstring
+ airflow/jobs/backfill_job_runner.py:341:21: DOC402 `yield` is not documented in docstring
+ airflow/metrics/otel_logger.py:374:9: DOC402 `yield` is not documented in docstring
+ airflow/models/abstractoperator.py:289:13: DOC402 `yield` is not documented in docstring
+ airflow/models/abstractoperator.py:314:17: DOC402 `yield` is not documented in docstring
+ airflow/models/abstractoperator.py:328:17: DOC402 `yield` is not documented in docstring
+ airflow/models/abstractoperator.py:357:17: DOC402 `yield` is not documented in docstring
+ airflow/models/abstractoperator.py:370:17: DOC402 `yield` is not documented in docstring
+ airflow/models/abstractoperator.py:397:9: DOC402 `yield` is not documented in docstring
+ airflow/models/dag.py:1241:17: DOC402 `yield` is not documented in docstring
+ airflow/models/dag.py:3648:21: DOC402 `yield` is not documented in docstring
+ airflow/models/dagrun.py:1405:13: DOC402 `yield` is not documented in docstring
+ airflow/models/dagrun.py:1502:13: DOC402 `yield` is not documented in docstring
+ airflow/models/dagrun.py:981:21: DOC402 `yield` is not documented in docstring
+ airflow/models/mappedoperator.py:862:13: DOC402 `yield` is not documented in docstring
+ airflow/models/taskinstance.py:2608:21: DOC402 `yield` is not documented in docstring
+ airflow/models/taskinstance.py:383:9: DOC402 `yield` is not documented in docstring
+ airflow/models/xcom_arg.py:113:13: DOC402 `yield` is not documented in docstring
+ airflow/operators/branch.py:65:17: DOC402 `yield` is not documented in docstring
+ airflow/providers/airbyte/triggers/airbyte.py:75:21: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/hooks/eks.py:609:13: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/hooks/logs.py:142:13: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/hooks/logs.py:236:21: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/hooks/s3.py:506:21: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/hooks/sagemaker.py:1465:17: 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/amazon/aws/transfers/sql_to_s3.py:221:13: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/triggers/batch.py:175:25: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/triggers/redshift_cluster.py:313:21: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/triggers/s3.py:100:29: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/triggers/s3.py:207:25: DOC402 `yield` is not documented in docstring
+ airflow/providers/amazon/aws/triggers/sagemaker.py:277:21: DOC402 `yield` is not documented in docstring
+ airflow/providers/apache/beam/triggers/beam.py:117:13: DOC402 `yield` is not documented in docstring
... 128 additional changes omitted for project

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

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

+ scripts/cancel_github_workflows.py:81:9: DOC402 `yield` is not documented in docstring
+ superset/commands/database/uploaders/columnar_reader.py:100:29: DOC402 `yield` is not documented in docstring
+ superset/distributed_lock/__init__.py:75:5: DOC402 `yield` is not documented in docstring
+ superset/extensions/metadb.py:415:17: DOC402 `yield` is not documented in docstring
+ superset/migrations/shared/utils.py:156:13: DOC402 `yield` is not documented in docstring
+ superset/models/core.py:445:13: DOC402 `yield` is not documented in docstring
+ superset/sql_parse.py:1557:17: DOC402 `yield` is not documented in docstring
+ superset/utils/core.py:1410:13: DOC402 `yield` is not documented in docstring
+ superset/utils/decorators.py:150:9: DOC402 `yield` is not documented in docstring
+ superset/utils/log.py:270:9: DOC402 `yield` is not documented in docstring
... 16 additional changes omitted for project

bokeh/bokeh (+5 -0 violations, +0 -0 fixes)

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

+ examples/basic/data/server_sent_events_source.py:60:13: DOC402 `yield` is not documented in docstring
+ src/bokeh/core/query.py:72:1: DOC403 Docstring has a "Yields" section but the function doesn't yield anything
+ src/bokeh/layouts.py:672:9: DOC402 `yield` is not documented in docstring
+ src/bokeh/util/dataclasses.py:73:17: DOC402 `yield` is not documented in docstring
+ tests/support/plugins/selenium.py:114:5: DOC402 `yield` is not documented in docstring

zulip/zulip (+6 -0 violations, +0 -0 fixes)

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

+ zerver/data_import/slack.py:850:9: DOC402 `yield` is not documented in docstring
+ zerver/lib/context_managers.py:45:13: DOC402 `yield` is not documented in docstring
+ zerver/lib/test_helpers.py:224:9: DOC402 `yield` is not documented in docstring
+ zerver/lib/test_helpers.py:234:13: DOC402 `yield` is not documented in docstring
+ zerver/lib/user_groups.py:183:9: DOC402 `yield` is not documented in docstring
+ zerver/tests/test_events.py:358:13: DOC402 `yield` is not documented in docstring

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
DOC402 204 204 0 0 0
DOC403 1 1 0 0 0

@charliermarsh charliermarsh added docstring Related to docstring linting or formatting preview Related to preview mode features labels Jul 29, 2024
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you and sorry for the wait. @AlexWaygood can you think of any other cases where a function would yield other than yield and yield from. If not, let's mege :)

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks great to me -- thanks @augustelalande!

@AlexWaygood
Copy link
Member

@augustelalande -- do you know exactly what the convention is with functions decorated with @contextlib.contextmanager? These functions contain yield statements but the decorator transforms the function so that it's no longer a generator function.

I can't immediately find anything about contextmanagers in the Sphinx or numpydoc docs, so I assume they're treated just like other functions with yields in them -- which makes sense, I think. But just checking you're not aware of any specific conventions regarding contextmanagers?

@augustelalande
Copy link
Contributor Author

@AlexWaygood lol nice edge case. I'm not really sure, I couldn't find anything either. Trying to debate in my head, I'm pretty 50/50, both having and not having the violation make sense.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's ship it. Thanks again!

@AlexWaygood AlexWaygood merged commit 94d817e into astral-sh:main Aug 2, 2024
20 checks passed
@augustelalande augustelalande deleted the doc4xx branch August 2, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docstring Related to docstring linting or formatting preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants