Allow overriding pydocstyle convention rules #8586

alanhdu · 2023-11-09T17:42:04Z

Summary

This fixes #2606 by moving where we apply the convention ignores -- instead of applying that at the very end, e track, we now track which rules have been specifically enabled (via Specificity::Rule). If they have, then we do not apply the docstring overrides at the end.

Test Plan

Added unit tests to ruff_workspace and an integration test to ruff_cli

github-actions · 2023-11-09T17:56:00Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

charliermarsh · 2023-11-10T01:38:03Z

This looks good to me, thanks for putting it together! I'd suggest adding a test or two to crates/ruff_cli/tests/integration_test.rs which can roughly mirror the way you tested it on the CLI, but with snapshot testing.

charliermarsh · 2023-11-10T01:38:46Z

(I think the implementation is fine. We already special-case conventions here, so I don't mind tracking a bit of extra state for it.)

charliermarsh · 2023-11-10T01:37:29Z

crates/ruff_workspace/src/configuration.rs

@@ -780,6 +791,8 @@ impl LintConfiguration {
                {
                    carryover_ignores = Some(&selection.ignore);
                }
+
+                docstring_overrides = docstring_override_updates;


I think it could be fine to omit docstring_override_updates and just add to docstring_overrides whenever we see a direct selector. In other words, I don't know if this "reset" behavior is important for the purpose of these overrides. I'm trying to think of a case where this would matter.

As an example of why this exists (to the best of my memory): imagine you have select=D and ignore=D401 in your configuration file. If you then pass --select D4, we want to ignore the ignore. So we treat a --select as resetting the selection chain.

Eh, thinking about it further, what you have here is probably right. Imagine you have select=D415 in your configuration file, and then you run with --select=D. We probably still want to enforce the configuration override for D415 in that case?

Hm... I was actually thinking about this in terms of having multiple ruff.tomls (e.g. in a monorepo or something). In that case, I think the intended behavior is that select "clears" the state of the "parent" ruff.tomls, and personally I think it'd be confusing if the docstring conventions were the only counter-example to that.

Makes sense.

alanhdu · 2023-11-10T14:26:04Z

Alright! Added some unit tests and an integration test, so this should be ready for review!

codspeed-hq · 2023-11-10T14:33:08Z

CodSpeed Performance Report

Merging #8586 will not alter performance

_{Comparing alanhdu:alan/override (e18d884) with main (a7dbe9d)}

Summary

✅ 25 untouched benchmarks

alanhdu · 2023-11-10T14:36:15Z

crates/ruff_cli/tests/integration_test.rs

+    success: false
+    exit_code: 1
+    ----- stdout -----
+    -:2:5: D417 Missing argument description in the docstring for `log`: `base`


FWIW, D417 was the rule I was surprised was disabled in the numpy convention. It looks like there's specific code written to handle some of the quirks (e.g.

ruff/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs

Line 1842 in 036b6bc

// Notably, NumPy lets you put multiple parameters of the same type on the same

), so it seemed odd that it was disabled.

alanhdu · 2023-11-10T14:36:35Z

crates/ruff_workspace/src/options.rs

-    /// As conventions force-disable all rules not included in the convention,
-    /// enabling _additional_ rules on top of a convention is currently
-    /// unsupported.
+    /// If you must, you can enable rules not included in a convention by


Is there another place I should update the docs, or is this the right place?

This is the right place :)

alanhdu · 2023-11-10T15:16:55Z

crates/ruff_cli/tests/integration_test.rs

+"#;
+
+    // If we only select the prefix, then everything passes
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))


FWIW, I found some of this to be a bit repetitive -- would you be open to a PR that tries to refactor some of this Command construction into a custom builder struct?

Sure, I'm open to the idea!

charliermarsh

This is great -- thanks for taking it on!

charliermarsh · 2023-11-10T18:42:26Z

Appreciate all the tests, thanks for following up there.

alanhdu mentioned this pull request Nov 9, 2023

Allow explicit select of docstring rule to override configured convention #2606

Closed

Allow overriding pydocstyle convention rules

1db79e4

alanhdu force-pushed the alan/override branch from df3c320 to 1db79e4 Compare November 9, 2023 19:25

charliermarsh self-requested a review November 10, 2023 01:38

charliermarsh added bug Something isn't working docstring Related to docstring linting or formatting labels Nov 10, 2023

charliermarsh reviewed Nov 10, 2023

View reviewed changes

alanhdu marked this pull request as ready for review November 10, 2023 14:24

alanhdu commented Nov 10, 2023

View reviewed changes

alanhdu added 3 commits November 10, 2023 10:16

Add unit tests

d337f66

Add integration test

9e8e604

Update docs

7cba23f

alanhdu force-pushed the alan/override branch from ae8eb2b to 7cba23f Compare November 10, 2023 15:16

alanhdu commented Nov 10, 2023

View reviewed changes

charliermarsh added 2 commits November 10, 2023 13:35

Merge branch 'main' into alan/override

6aca7e2

Update schema

519de68

charliermarsh approved these changes Nov 10, 2023

View reviewed changes

Tweak test

a29b1a8

charliermarsh enabled auto-merge (squash) November 10, 2023 18:42

Tweak docs

e18d884

charliermarsh merged commit 5a1a8be into astral-sh:main Nov 10, 2023
16 checks passed

alanhdu deleted the alan/override branch November 10, 2023 18:55

miccal mentioned this pull request Nov 17, 2023

ruff 0.1.6 Homebrew/homebrew-core#154716

Merged

tlambert03 mentioned this pull request Nov 20, 2023

Simplify ruff docs config pydev-guide/pyrepo-copier#23

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow overriding pydocstyle convention rules #8586

Allow overriding pydocstyle convention rules #8586

alanhdu commented Nov 9, 2023 •

edited

Loading

github-actions bot commented Nov 9, 2023 •

edited

Loading

charliermarsh commented Nov 10, 2023

charliermarsh commented Nov 10, 2023

charliermarsh Nov 10, 2023

alanhdu Nov 10, 2023

charliermarsh Nov 10, 2023

alanhdu commented Nov 10, 2023

codspeed-hq bot commented Nov 10, 2023 •

edited

Loading

alanhdu Nov 10, 2023

alanhdu Nov 10, 2023

charliermarsh Nov 10, 2023

alanhdu Nov 10, 2023

charliermarsh Nov 10, 2023

charliermarsh left a comment

charliermarsh commented Nov 10, 2023

Allow overriding pydocstyle convention rules #8586

Allow overriding pydocstyle convention rules #8586

Conversation

alanhdu commented Nov 9, 2023 • edited Loading

Summary

Test Plan

github-actions bot commented Nov 9, 2023 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

charliermarsh commented Nov 10, 2023

charliermarsh commented Nov 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanhdu commented Nov 10, 2023

codspeed-hq bot commented Nov 10, 2023 • edited Loading

Merging #8586 will not alter performance

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh left a comment

Choose a reason for hiding this comment

charliermarsh commented Nov 10, 2023

alanhdu commented Nov 9, 2023 •

edited

Loading

github-actions bot commented Nov 9, 2023 •

edited

Loading

`ruff-ecosystem` results

codspeed-hq bot commented Nov 10, 2023 •

edited

Loading