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

[pydocstyle] Skip leading whitespace for D403 #14963

Merged
merged 10 commits into from
Dec 16, 2024

Conversation

dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented Dec 13, 2024

This PR introduces three changes to D403, which has to do with capitalizing the first word in a docstring.

  1. The diagnostic and fix now skip leading whitespace when determining what counts as "the first word".
  2. The name has been changed to first-word-uncapitalized from first-line-capitalized, for both clarity and compliance with our rule naming policy.
  3. The diagnostic message and documentation has been modified slightly to reflect this.

Closes #14890

@dylwil3
Copy link
Collaborator Author

dylwil3 commented Dec 13, 2024

Two questions for reviewers:

  1. I used trim_start (which trims unicode-defined whitespace) rather than trim_whitespace_start (which trims "Python" whitespace) since we were inside a docstring. But let me know if I should change that.
  2. Is this enough of a rule change that it should not be merged until a minor release? (Added later: Haven't looked at them all, but the ecosystem results look correct to me. There are a lot of new violations, so maybe that's a factor in answering this question.)

Copy link
Contributor

github-actions bot commented Dec 13, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+48 -11 violations, +0 -0 fixes in 8 projects; 47 projects unchanged)

RasaHQ/rasa (+5 -4 violations, +0 -0 fixes)

+ tests/core/test_broker.py:309:5: D403 [*] First word of the docstring should be capitalized: `tests` -> `Tests`
+ tests/nlu/classifiers/test_diet_classifier.py:1025:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:1025:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`
+ tests/nlu/classifiers/test_diet_classifier.py:504:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:504:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`
+ tests/nlu/classifiers/test_diet_classifier.py:953:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:953:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`
+ tests/nlu/classifiers/test_diet_classifier.py:995:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:995:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`

PlasmaPy/PlasmaPy (+2 -0 violations, +0 -0 fixes)

+ src/plasmapy/diagnostics/thomson.py:684:5: D403 [*] First word of the docstring should be capitalized: `lmfit` -> `Lmfit`
+ tests/simulation/test_particle_tracker.py:568:9: D403 [*] First word of the docstring should be capitalized: `fsolve` -> `Fsolve`

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

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

+ providers/src/airflow/providers/weaviate/hooks/weaviate.py:751:9: D403 [*] First word of the docstring should be capitalized: `create` -> `Create`

apache/superset (+29 -4 violations, +0 -0 fixes)

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

+ superset/commands/importers/v1/__init__.py:115:9: D403 [*] First word of the docstring should be capitalized: `check` -> `Check`
- superset/commands/importers/v1/__init__.py:115:9: D403 [*] First word of the first line should be capitalized: `check` -> `Check`
+ superset/common/utils/query_cache_manager.py:204:9: D403 [*] First word of the docstring should be capitalized: `set` -> `Set`
+ superset/common/utils/time_range_utils.py:52:5: D403 [*] First word of the docstring should be capitalized: `this` -> `This`
+ superset/daos/tag.py:105:9: D403 [*] First word of the docstring should be capitalized: `deletes` -> `Deletes`
+ superset/daos/tag.py:121:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:136:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:146:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:171:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:82:9: D403 [*] First word of the docstring should be capitalized: `deletes` -> `Deletes`
+ superset/models/helpers.py:455:9: D403 [*] First word of the docstring should be capitalized: `object` -> `Object`
- superset/models/helpers.py:455:9: D403 [*] First word of the first line should be capitalized: `object` -> `Object`
+ superset/sql_lab.py:158:5: D403 [*] First word of the docstring should be capitalized: `attempts` -> `Attempts`
- superset/sql_lab.py:158:5: D403 [*] First word of the first line should be capitalized: `attempts` -> `Attempts`
+ superset/sql_parse.py:147:5: D403 [*] First word of the docstring should be capitalized: `parse` -> `Parse`
+ superset/utils/json.py:234:5: D403 [*] First word of the docstring should be capitalized: `deserializable` -> `Deserializable`
+ superset/utils/pandas_postprocessing/resample.py:32:5: D403 [*] First word of the docstring should be capitalized: `support` -> `Support`
+ tests/integration_tests/import_export_tests.py:210:9: D403 [*] First word of the docstring should be capitalized: `only` -> `Only`
- tests/integration_tests/import_export_tests.py:210:9: D403 [*] First word of the first line should be capitalized: `only` -> `Only`
+ tests/integration_tests/migrations/f1410ed7ec95_migrate_native_filters_to_new_schema__tests.py:80:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f1410ed7ec95_migrate_native_filters_to_new_schema__tests.py:91:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:175:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:184:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:197:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:206:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:216:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:225:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
... 6 additional changes omitted for project

bokeh/bokeh (+4 -1 violations, +0 -0 fixes)

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

+ src/bokeh/command/subcommands/info.py:84:5: D403 [*] First word of the docstring should be capitalized: `helper` -> `Helper`
+ src/bokeh/sphinxext/bokehjs_content.py:132:9: D403 [*] First word of the docstring should be capitalized: `this` -> `This`
- src/bokeh/sphinxext/bokehjs_content.py:132:9: D403 [*] First word of the first line should be capitalized: `this` -> `This`
+ tests/support/util/screenshot.py:87:5: D403 [*] First word of the docstring should be capitalized: `wait` -> `Wait`
+ tests/unit/bokeh/test_events.py:144:5: D403 [*] First word of the docstring should be capitalized: `model` -> `Model`

latchbio/latch (+1 -1 violations, +0 -0 fixes)

+ src/latch/resources/tasks.py:283:5: D403 [*] First word of the docstring should be capitalized: `any` -> `Any`
- src/latch/resources/tasks.py:283:5: D403 [*] First word of the first line should be capitalized: `any` -> `Any`

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


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

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

+ zerver/lib/markdown/fenced_code.py:562:9: D403 [*] First word of the docstring should be capitalized: `basic` -> `Basic`
- zerver/lib/markdown/fenced_code.py:562:9: D403 [*] First word of the first line should be capitalized: `basic` -> `Basic`
+ zerver/lib/message_cache.py:354:9: D403 [*] First word of the docstring should be capitalized: `row` -> `Row`
+ zerver/tests/test_decorators.py:817:9: D403 [*] First word of the docstring should be capitalized: `logging` -> `Logging`
+ zerver/tests/test_decorators.py:830:9: D403 [*] First word of the docstring should be capitalized: `logging` -> `Logging`
+ zerver/tests/test_message_fetch.py:3894:9: D403 [*] First word of the docstring should be capitalized: `narrow` -> `Narrow`
+ zproject/backends.py:3075:5: D403 [*] First word of the docstring should be capitalized: `wantMessagesSigned` -> `WantMessagesSigned`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
D403 59 48 11 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+48 -11 violations, +0 -0 fixes in 7 projects; 48 projects unchanged)

RasaHQ/rasa (+5 -4 violations, +0 -0 fixes)

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

+ tests/core/test_broker.py:309:5: D403 [*] First word of the docstring should be capitalized: `tests` -> `Tests`
+ tests/nlu/classifiers/test_diet_classifier.py:1025:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:1025:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`
+ tests/nlu/classifiers/test_diet_classifier.py:504:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:504:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`
+ tests/nlu/classifiers/test_diet_classifier.py:953:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:953:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`
+ tests/nlu/classifiers/test_diet_classifier.py:995:5: D403 [*] First word of the docstring should be capitalized: `test` -> `Test`
- tests/nlu/classifiers/test_diet_classifier.py:995:5: D403 [*] First word of the first line should be capitalized: `test` -> `Test`

PlasmaPy/PlasmaPy (+2 -0 violations, +0 -0 fixes)

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

+ src/plasmapy/diagnostics/thomson.py:684:5: D403 [*] First word of the docstring should be capitalized: `lmfit` -> `Lmfit`
+ tests/simulation/test_particle_tracker.py:568:9: D403 [*] First word of the docstring should be capitalized: `fsolve` -> `Fsolve`

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

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

+ providers/src/airflow/providers/weaviate/hooks/weaviate.py:751:9: D403 [*] First word of the docstring should be capitalized: `create` -> `Create`

apache/superset (+29 -4 violations, +0 -0 fixes)

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

+ superset/commands/importers/v1/__init__.py:115:9: D403 [*] First word of the docstring should be capitalized: `check` -> `Check`
- superset/commands/importers/v1/__init__.py:115:9: D403 [*] First word of the first line should be capitalized: `check` -> `Check`
+ superset/common/utils/query_cache_manager.py:204:9: D403 [*] First word of the docstring should be capitalized: `set` -> `Set`
+ superset/common/utils/time_range_utils.py:52:5: D403 [*] First word of the docstring should be capitalized: `this` -> `This`
+ superset/daos/tag.py:105:9: D403 [*] First word of the docstring should be capitalized: `deletes` -> `Deletes`
+ superset/daos/tag.py:121:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:136:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:146:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:171:9: D403 [*] First word of the docstring should be capitalized: `returns` -> `Returns`
+ superset/daos/tag.py:82:9: D403 [*] First word of the docstring should be capitalized: `deletes` -> `Deletes`
+ superset/models/helpers.py:455:9: D403 [*] First word of the docstring should be capitalized: `object` -> `Object`
- superset/models/helpers.py:455:9: D403 [*] First word of the first line should be capitalized: `object` -> `Object`
+ superset/sql_lab.py:158:5: D403 [*] First word of the docstring should be capitalized: `attempts` -> `Attempts`
- superset/sql_lab.py:158:5: D403 [*] First word of the first line should be capitalized: `attempts` -> `Attempts`
+ superset/sql_parse.py:147:5: D403 [*] First word of the docstring should be capitalized: `parse` -> `Parse`
+ superset/utils/json.py:234:5: D403 [*] First word of the docstring should be capitalized: `deserializable` -> `Deserializable`
+ superset/utils/pandas_postprocessing/resample.py:32:5: D403 [*] First word of the docstring should be capitalized: `support` -> `Support`
+ tests/integration_tests/import_export_tests.py:210:9: D403 [*] First word of the docstring should be capitalized: `only` -> `Only`
- tests/integration_tests/import_export_tests.py:210:9: D403 [*] First word of the first line should be capitalized: `only` -> `Only`
+ tests/integration_tests/migrations/f1410ed7ec95_migrate_native_filters_to_new_schema__tests.py:80:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f1410ed7ec95_migrate_native_filters_to_new_schema__tests.py:91:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:175:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:184:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:197:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:206:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:216:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
+ tests/integration_tests/migrations/f84fde59123a_update_charts_with_old_time_comparison__test.py:225:5: D403 [*] First word of the docstring should be capitalized: `ensure` -> `Ensure`
... 6 additional changes omitted for project

bokeh/bokeh (+4 -1 violations, +0 -0 fixes)

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

+ src/bokeh/command/subcommands/info.py:84:5: D403 [*] First word of the docstring should be capitalized: `helper` -> `Helper`
+ src/bokeh/sphinxext/bokehjs_content.py:132:9: D403 [*] First word of the docstring should be capitalized: `this` -> `This`
- src/bokeh/sphinxext/bokehjs_content.py:132:9: D403 [*] First word of the first line should be capitalized: `this` -> `This`
+ tests/support/util/screenshot.py:87:5: D403 [*] First word of the docstring should be capitalized: `wait` -> `Wait`
+ tests/unit/bokeh/test_events.py:144:5: D403 [*] First word of the docstring should be capitalized: `model` -> `Model`

latchbio/latch (+1 -1 violations, +0 -0 fixes)

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

+ src/latch/resources/tasks.py:283:5: D403 [*] First word of the docstring should be capitalized: `any` -> `Any`
- src/latch/resources/tasks.py:283:5: D403 [*] First word of the first line should be capitalized: `any` -> `Any`

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

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

+ zerver/lib/markdown/fenced_code.py:562:9: D403 [*] First word of the docstring should be capitalized: `basic` -> `Basic`
- zerver/lib/markdown/fenced_code.py:562:9: D403 [*] First word of the first line should be capitalized: `basic` -> `Basic`
+ zerver/lib/message_cache.py:354:9: D403 [*] First word of the docstring should be capitalized: `row` -> `Row`
+ zerver/tests/test_decorators.py:817:9: D403 [*] First word of the docstring should be capitalized: `logging` -> `Logging`
+ zerver/tests/test_decorators.py:830:9: D403 [*] First word of the docstring should be capitalized: `logging` -> `Logging`
+ zerver/tests/test_message_fetch.py:3894:9: D403 [*] First word of the docstring should be capitalized: `narrow` -> `Narrow`
+ zproject/backends.py:3075:5: D403 [*] First word of the docstring should be capitalized: `wantMessagesSigned` -> `WantMessagesSigned`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
D403 59 48 11 0 0

@dylwil3 dylwil3 added rule Implementing or modifying a lint rule breaking Breaking API change labels Dec 13, 2024
@MichaReiser
Copy link
Member

The change from first-line to first-word makes sense to me. Would you mind doing a quick check if this is "aligned" with what other docstyle rules do? For example D400, D401 also only consider the first line. We might need to review the rules more holistically

I don't think we should change the diagnostic location. Or at least, I suggest to do it as a separate PR because it's technically a breaking change and can invalidate all noqa comments.

@dylwil3
Copy link
Collaborator Author

dylwil3 commented Dec 16, 2024

Would you mind doing a quick check if this is "aligned" with what other docstyle rules do? For example D400, D401 also only consider the first line. We might need to review the rules more holistically

Good call!

These seem like the rules that should ignore leading whitespace, and it looks like their implementations indeed do (except for the one under review):

  • ends-in-period (D400)
  • non-imperative-mood (D401)
  • no-signature (D402)
  • first-line-capitalized (D403) [covered in this PR]
  • docstring-starts-with-this (D404)
  • ends-in-punctuation (D415)

I don't think we should change the diagnostic location. Or at least, I suggest to do it as a separate PR because it's technically a breaking change and can invalidate all noqa comments.

Reverted.

@dylwil3 dylwil3 removed the breaking Breaking API change label Dec 16, 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.

Thanks

@dylwil3 dylwil3 merged commit 6a5eff6 into astral-sh:main Dec 16, 2024
21 checks passed
dcreager added a commit that referenced this pull request Dec 16, 2024
* main: (25 commits)
  [`pydocstyle`] Skip leading whitespace for `D403` (#14963)
  Update pre-commit dependencies (#15008)
  Check diagnostic refresh support from client capability (#15014)
  Update Rust crate colored to v2.2.0 (#15010)
  Update dependency monaco-editor to v0.52.2 (#15006)
  Update Rust crate thiserror to v2.0.7 (#15005)
  Update Rust crate serde to v1.0.216 (#15004)
  Update Rust crate libc to v0.2.168 (#15003)
  Update Rust crate fern to v0.7.1 (#15002)
  Update Rust crate chrono to v0.4.39 (#15001)
  Update Rust crate bstr to v1.11.1 (#15000)
  Update NPM Development dependencies (#14999)
  Update dependency ruff to v0.8.3 (#15007)
  Pin mdformat plugins in pre-commit (#14992)
  Use stripping block (`|-`) for page descriptions (#14980)
  [`perflint`] Fix panic in `perf401` (#14971)
  Improve the documentation of E201/E202 (#14983)
  [ruff_python_ast] Add name and default functions to TypeParam. (#14964)
  [red-knot] Emit an error if a bare `Annotated` or `Literal` is used in a type expression (#14973)
  [red-knot] Fix bugs relating to assignability of dynamic `type[]` types (#14972)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

D403 first-line-capitalized lint not alerting when the first line is after a newline
2 participants