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

[Ruff 0.6] Stabilise 9 pylint rules #12857

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Aug 13, 2024

Summary

Stabilise the following rules for Ruff 0.6:

  • singledispatch-method (PLE1519)
  • singledispatchmethod-function (PLE1520)
  • bad-staticmethod-argument (PLW0211)
  • if-stmt-min-max (PLR1730)
  • invalid-bytes-return-type (PLE0308)
  • invalid-hash-return-type (PLE0309)
  • invalid-index-return-type (PLE0305)
  • invalid-length-return-type (E303)
  • non-augmented-assignment (PLR6104)
  • self-or-cls-assignment (PLW0642)

These rules all seem fairly uncontroversial, they've all been in preview for >3 months (and haven't changed much in a while), and none of them have any significant issues open about them that should block stabilisation IMO.

Test Plan

cargo test, but the ecoystem report will be the real judge...

Copy link
Contributor

github-actions bot commented Aug 13, 2024

ruff-ecosystem results

Linter (stable)

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

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

+ src/plasmapy/particles/particle_class.py:845:21: PLR1730 [*] Replace `if` statement with `max_charge = max(charge, max_charge)`

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

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

+ airflow/cli/commands/kubernetes_command.py:91:5: PLR1730 [*] Replace `if` statement with `min_pending_minutes = max(min_pending_minutes, 5)`
+ airflow/models/taskinstance.py:2652:13: PLR1730 [*] Replace `if` statement with `min_backoff = max(min_backoff, 1)`
+ airflow/providers/cncf/kubernetes/cli/kubernetes_command.py:84:5: PLR1730 [*] Replace `if` statement with `min_pending_minutes = max(min_pending_minutes, 5)`
+ tests/cluster_policies/__init__.py:103:5: PLR1730 [*] Replace `if` statement with `min` call
+ tests/secrets/test_cache.py:43:25: PLW0211 First argument of a static method should not be named `self`

aws/aws-sam-cli (+4 -0 violations, +0 -0 fixes)

+ samcli/lib/observability/cw_logs/cw_log_puller.py:136:17: PLR1730 [*] Replace `if` statement with `max` call
+ samcli/lib/observability/xray_traces/xray_event_puller.py:163:21: PLR1730 [*] Replace `if` statement with `max` call
+ samcli/lib/observability/xray_traces/xray_events.py:52:13: PLR1730 [*] Replace `if` statement with `max` call
+ samcli/lib/observability/xray_traces/xray_events.py:87:13: PLR1730 [*] Replace `if` statement with `max` call

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

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

+ src/bokeh/events.py:182:9: PLW0642 Confusing assignment to `cls` argument in class method

milvus-io/pymilvus (+3 -0 violations, +0 -0 fixes)

+ pymilvus/bulk_writer/buffer.py:270:13: PLR1730 [*] Replace `if` statement with `max` call
+ pymilvus/bulk_writer/buffer.py:272:13: PLR1730 [*] Replace `if` statement with `min` call
+ pymilvus/orm/iterator.py:54:9: PLR1730 [*] Replace `if` statement with `min` call

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

+ pandas/core/arrays/categorical.py:572:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1065:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1080:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1081:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1109:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1118:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1129:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1131:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1136:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1185:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1187:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1203:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1260:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1274:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1480:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1699:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:2131:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:2153:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:456:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:778:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:979:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/frame.py:7827:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/frame.py:7840:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/frame.py:8183:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/frame.py:8195:21: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/frame.py:8235:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:3468:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:3629:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:7992:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:7995:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:7998:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:9235:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:9242:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/generic.py:9245:17: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/groupby/grouper.py:258:13: PLW0642 Confusing assignment to `cls` argument in class method
+ pandas/core/indexes/base.py:1412:13: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/indexes/base.py:2256:9: PLW0642 Confusing assignment to `self` argument in instance method
+ pandas/core/indexes/base.py:3067:13: PLW0642 Confusing assignment to `self` argument in instance method
... 13 additional changes omitted for project

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

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

+ zerver/tests/test_openapi.py:321:13: PLR1730 [*] Replace `if` statement with `val = min(v, val)`

Changes by rule (3 rules affected)

code total + violation - violation + fix - fix
PLW0642 52 52 0 0 0
PLR1730 13 13 0 0 0
PLW0211 1 1 0 0 0

Linter (preview)

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

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

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

+ src/bokeh/events.py:182:9: PLW0642 Confusing assignment to `cls` argument in class method
- src/bokeh/events.py:182:9: PLW0642 Invalid assignment to `cls` argument in class method

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

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

+ pandas/core/arrays/categorical.py:572:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/categorical.py:572:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1065:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1065:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1080:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1080:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1081:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1081:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1109:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1109:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1118:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1118:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1129:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1129:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1131:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1131:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1136:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1136:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1185:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1185:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1187:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1187:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1203:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1203:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1260:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1260:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1274:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1274:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1480:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1480:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:1699:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:1699:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:2131:17: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:2131:17: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:2153:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:2153:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:456:17: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:456:17: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:778:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:778:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/arrays/datetimelike.py:979:13: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/arrays/datetimelike.py:979:13: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/frame.py:7827:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/frame.py:7827:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/frame.py:7840:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/frame.py:7840:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/frame.py:8183:9: PLW0642 Confusing assignment to `self` argument in instance method
- pandas/core/frame.py:8183:9: PLW0642 Invalid assignment to `self` argument in instance method
+ pandas/core/frame.py:8195:21: PLW0642 Confusing assignment to `self` argument in instance method
... 53 additional changes omitted for project

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLW0642 104 52 52 0 0

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Aug 13, 2024

The vast majority of the ecosystem hit here are PLR6104. I'll split that one out into its own PR so that it can be considered in isolation.

(Edit: this has now been done; the PR for that is #12858)

@AlexWaygood AlexWaygood changed the title [Ruff 0.6] Stabilise 10 pylint rules [Ruff 0.6] Stabilise 9 pylint rules Aug 13, 2024
@AlexWaygood
Copy link
Member Author

There's a lot of PLW0642 hits on pandas, but I think they're all true positives. pandas apparently does a lot of self = foo assignments, but I think these could easily be replaced with other variable names, which would improve the readability of the code.

@MichaReiser
Copy link
Member

LGTM. My only feedback for PLW0642 is that I find the message: Invalid assignment to somewhat confusing. I'm not familiar enough with Python but the assignment itself isn't invalid. It's just not recommended to change the cls or self variable but I don't think it breaks anything, it's just unexpected.

@AlexWaygood
Copy link
Member Author

LGTM. My only feedback for PLW0642 is that I find the message: Invalid assignment to somewhat confusing. I'm not familiar enough with Python but the assignment itself isn't invalid. It's just not recommended to change the cls or self variable but I don't think it breaks anything, it's just unexpected.

Yeah, that's a great point. I think I'll change it to "confusing". It's not an invalid assignment; you could just write the code in a clearer/less confusing way by using a different variable name.

@AlexWaygood AlexWaygood merged commit 72707b3 into ruff-0.6 Aug 13, 2024
20 checks passed
@AlexWaygood AlexWaygood deleted the pylint-stabilisations branch August 13, 2024 13:38
Comment on lines +60 to +63

fn fix_title(&self) -> Option<String> {
Some("Consider using a different variable name".to_string())
}
Copy link
Member

Choose a reason for hiding this comment

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

This is clever ;)

Comment on lines 55 to 57
format!(
"Invalid assignment to `{}` argument in {method_type} method",
"Confusing assignment to `{}` argument in {method_type} method",
method_type.arg_name(),
Copy link
Member

Choose a reason for hiding this comment

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

Another option could be "Re-assigned {} argument in {method_type} method"

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, that's better. Much more descriptive. I'll make another PR 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants