From a0dc90c1d39bf7865daa4139ccd4f1f07e208f99 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Fri, 10 Feb 2023 22:40:32 +0100 Subject: [PATCH] autofixes for PD003, PD004, PD008, PD009 and PD011 --- README.md | 10 ++--- .../test/fixtures/pandas_vet/fixes.py | 14 ++++++ crates/ruff/src/rules/pandas_vet/fixes.rs | 18 +++++++- crates/ruff/src/rules/pandas_vet/mod.rs | 5 +++ .../src/rules/pandas_vet/rules/check_attr.rs | 43 ++++++++++++++++--- .../src/rules/pandas_vet/rules/check_call.rs | 36 +++++++++++++--- ...es__pandas_vet__tests__PD003_fixes.py.snap | 23 ++++++++++ ...es__pandas_vet__tests__PD004_fixes.py.snap | 23 ++++++++++ ...es__pandas_vet__tests__PD008_fixes.py.snap | 23 ++++++++++ ...es__pandas_vet__tests__PD009_fixes.py.snap | 23 ++++++++++ ...es__pandas_vet__tests__PD011_fixes.py.snap | 23 ++++++++++ 11 files changed, 221 insertions(+), 20 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pandas_vet/fixes.py create mode 100644 crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD003_fixes.py.snap create mode 100644 crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD004_fixes.py.snap create mode 100644 crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD008_fixes.py.snap create mode 100644 crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD009_fixes.py.snap create mode 100644 crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD011_fixes.py.snap diff --git a/README.md b/README.md index e8a578526c72d..b717bc8c92b14 100644 --- a/README.md +++ b/README.md @@ -1322,13 +1322,13 @@ For more, see [pandas-vet](https://pypi.org/project/pandas-vet/) on PyPI. | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | | PD002 | use-of-inplace-argument | `inplace=True` should be avoided; it has inconsistent behavior | 🛠 | -| PD003 | use-of-dot-is-null | `.isna` is preferred to `.isnull`; functionality is equivalent | | -| PD004 | use-of-dot-not-null | `.notna` is preferred to `.notnull`; functionality is equivalent | | +| PD003 | use-of-dot-is-null | `.isna` is preferred to `.isnull`; functionality is equivalent | 🛠 | +| PD004 | use-of-dot-not-null | `.notna` is preferred to `.notnull`; functionality is equivalent | 🛠 | | PD007 | use-of-dot-ix | `.ix` is deprecated; use more explicit `.loc` or `.iloc` | | -| PD008 | use-of-dot-at | Use `.loc` instead of `.at`. If speed is important, use numpy. | | -| PD009 | use-of-dot-iat | Use `.iloc` instead of `.iat`. If speed is important, use numpy. | | +| PD008 | use-of-dot-at | Use `.loc` instead of `.at`. If speed is important, use numpy. | 🛠 | +| PD009 | use-of-dot-iat | Use `.iloc` instead of `.iat`. If speed is important, use numpy. | 🛠 | | PD010 | use-of-dot-pivot-or-unstack | `.pivot_table` is preferred to `.pivot` or `.unstack`; provides same functionality | | -| PD011 | use-of-dot-values | Use `.to_numpy()` instead of `.values` | | +| PD011 | use-of-dot-values | Use `.to_numpy()` instead of `.values` | 🛠 | | PD012 | use-of-dot-read-table | `.read_csv` is preferred to `.read_table`; provides same functionality | | | PD013 | use-of-dot-stack | `.melt` is preferred to `.stack`; provides same functionality | | | PD015 | use-of-pd-merge | Use `.merge` method instead of `pd.merge` function. They have equivalent functionality. | | diff --git a/crates/ruff/resources/test/fixtures/pandas_vet/fixes.py b/crates/ruff/resources/test/fixtures/pandas_vet/fixes.py new file mode 100644 index 0000000000000..8f52473ea0a30 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pandas_vet/fixes.py @@ -0,0 +1,14 @@ +import pandas as pd + +df = pd.DataFrame() + +df.isnull() +df.isna() +df.notnull() +df.notna() +df.at[1] +df.loc[1] +df.iat[1] +df.iloc[1] +df.values +df.to_numpy() diff --git a/crates/ruff/src/rules/pandas_vet/fixes.rs b/crates/ruff/src/rules/pandas_vet/fixes.rs index 941147f42115b..ba4e57f2ee8c3 100644 --- a/crates/ruff/src/rules/pandas_vet/fixes.rs +++ b/crates/ruff/src/rules/pandas_vet/fixes.rs @@ -1,11 +1,11 @@ -use rustpython_parser::ast::{Expr, ExprKind, Keyword, Location}; +use rustpython_parser::ast::{Expr, ExprContext, ExprKind, Keyword, Location}; use crate::ast::helpers; use crate::ast::types::Range; use crate::autofix::apply_fix; use crate::autofix::helpers::remove_argument; use crate::fix::Fix; -use crate::source_code::Locator; +use crate::source_code::{Locator, Stylist}; fn match_name(expr: &Expr) -> Option<&str> { if let ExprKind::Call { func, .. } = &expr.node { @@ -70,3 +70,17 @@ pub fn fix_inplace_argument( None } } + +/// Replace attribute with `attr` +pub fn fix_attr(attr: &str, value: &Expr, stylist: &Stylist) -> String { + let stmt = Expr::new( + Location::default(), + Location::default(), + ExprKind::Attribute { + value: Box::new(value.clone()), + attr: attr.to_string(), + ctx: ExprContext::Store, + }, + ); + helpers::unparse_expr(&stmt, stylist) +} diff --git a/crates/ruff/src/rules/pandas_vet/mod.rs b/crates/ruff/src/rules/pandas_vet/mod.rs index 69dc1fe4c3e0f..c9b511d31556a 100644 --- a/crates/ruff/src/rules/pandas_vet/mod.rs +++ b/crates/ruff/src/rules/pandas_vet/mod.rs @@ -250,6 +250,11 @@ mod tests { } #[test_case(Rule::UseOfInplaceArgument, Path::new("PD002.py"); "PD002")] + #[test_case(Rule::UseOfDotIsNull, Path::new("fixes.py"); "PD003")] + #[test_case(Rule::UseOfDotNotNull, Path::new("fixes.py"); "PD004")] + #[test_case(Rule::UseOfDotAt, Path::new("fixes.py"); "PD008")] + #[test_case(Rule::UseOfDotIat, Path::new("fixes.py"); "PD009")] + #[test_case(Rule::UseOfDotValues, Path::new("fixes.py"); "PD011")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs b/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs index 33c394971aa20..594a091df4c21 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs @@ -3,9 +3,11 @@ use rustpython_parser::ast::{ExprKind, Located}; use crate::ast::types::{BindingKind, Range}; use crate::checkers::ast::Checker; +use crate::fix::Fix; use crate::registry::{Diagnostic, DiagnosticKind, Rule}; +use crate::rules::pandas_vet::fixes::fix_attr; use crate::rules::pandas_vet::helpers::is_dataframe_candidate; -use crate::violation::Violation; +use crate::violation::{AlwaysAutofixableViolation, Violation}; define_violation!( pub struct UseOfDotIx; @@ -20,31 +22,43 @@ impl Violation for UseOfDotIx { define_violation!( pub struct UseOfDotAt; ); -impl Violation for UseOfDotAt { +impl AlwaysAutofixableViolation for UseOfDotAt { #[derive_message_formats] fn message(&self) -> String { format!("Use `.loc` instead of `.at`. If speed is important, use numpy.") } + + fn autofix_title(&self) -> String { + format!("Replace `.at` with `.loc`") + } } define_violation!( pub struct UseOfDotIat; ); -impl Violation for UseOfDotIat { +impl AlwaysAutofixableViolation for UseOfDotIat { #[derive_message_formats] fn message(&self) -> String { format!("Use `.iloc` instead of `.iat`. If speed is important, use numpy.") } + + fn autofix_title(&self) -> String { + format!("Replace `.iat` with `.iloc`") + } } define_violation!( pub struct UseOfDotValues; ); -impl Violation for UseOfDotValues { +impl AlwaysAutofixableViolation for UseOfDotValues { #[derive_message_formats] fn message(&self) -> String { format!("Use `.to_numpy()` instead of `.values`") } + + fn autofix_title(&self) -> String { + format!("Replace `.values` with `.to_numpy()`") + } } pub fn check_attr( @@ -94,7 +108,22 @@ pub fn check_attr( } } - checker - .diagnostics - .push(Diagnostic::new(violation, Range::from_located(attr_expr))); + let mut diagnostic = Diagnostic::new(violation, Range::from_located(attr_expr)); + if checker.patch(diagnostic.kind.rule()) { + let replacement = match *diagnostic.kind.rule() { + Rule::UseOfDotAt => Some("loc"), + Rule::UseOfDotIat => Some("iloc"), + Rule::UseOfDotValues => Some("to_numpy()"), + _ => None, + }; + if let Some(replacement) = replacement { + diagnostic.amend(Fix::replacement( + fix_attr(replacement, value, checker.stylist), + attr_expr.location, + attr_expr.end_location.unwrap(), + )); + } + } + + checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/pandas_vet/rules/check_call.rs b/crates/ruff/src/rules/pandas_vet/rules/check_call.rs index dacd841d99c87..73380b4750e40 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/check_call.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/check_call.rs @@ -3,28 +3,38 @@ use rustpython_parser::ast::{ExprKind, Located}; use crate::ast::types::{BindingKind, Range}; use crate::checkers::ast::Checker; +use crate::fix::Fix; use crate::registry::{Diagnostic, DiagnosticKind, Rule}; +use crate::rules::pandas_vet::fixes::fix_attr; use crate::rules::pandas_vet::helpers::is_dataframe_candidate; -use crate::violation::Violation; +use crate::violation::{AlwaysAutofixableViolation, Violation}; define_violation!( pub struct UseOfDotIsNull; ); -impl Violation for UseOfDotIsNull { +impl AlwaysAutofixableViolation for UseOfDotIsNull { #[derive_message_formats] fn message(&self) -> String { format!("`.isna` is preferred to `.isnull`; functionality is equivalent") } + + fn autofix_title(&self) -> String { + format!("Replace `.isnull` with `.isna`") + } } define_violation!( pub struct UseOfDotNotNull; ); -impl Violation for UseOfDotNotNull { +impl AlwaysAutofixableViolation for UseOfDotNotNull { #[derive_message_formats] fn message(&self) -> String { format!("`.notna` is preferred to `.notnull`; functionality is equivalent") } + + fn autofix_title(&self) -> String { + format!("Replace `.notnull` with `.notna`") + } } define_violation!( @@ -102,7 +112,21 @@ pub fn check_call(checker: &mut Checker, func: &Located) { } } - checker - .diagnostics - .push(Diagnostic::new(violation, Range::from_located(func))); + let mut diagnostic = Diagnostic::new(violation, Range::from_located(func)); + if checker.patch(diagnostic.kind.rule()) { + let replacement = match *diagnostic.kind.rule() { + Rule::UseOfDotIsNull => Some("isna"), + Rule::UseOfDotNotNull => Some("notna"), + _ => None, + }; + if let Some(replacement) = replacement { + diagnostic.amend(Fix::replacement( + fix_attr(replacement, value, checker.stylist), + func.location, + func.end_location.unwrap(), + )); + } + } + + checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD003_fixes.py.snap b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD003_fixes.py.snap new file mode 100644 index 0000000000000..5f228fa8ed908 --- /dev/null +++ b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD003_fixes.py.snap @@ -0,0 +1,23 @@ +--- +source: crates/ruff/src/rules/pandas_vet/mod.rs +expression: diagnostics +--- +- kind: + UseOfDotIsNull: ~ + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 9 + fix: + content: + - df.isna + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 9 + parent: ~ + diff --git a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD004_fixes.py.snap b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD004_fixes.py.snap new file mode 100644 index 0000000000000..4439eeaae03f1 --- /dev/null +++ b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD004_fixes.py.snap @@ -0,0 +1,23 @@ +--- +source: crates/ruff/src/rules/pandas_vet/mod.rs +expression: diagnostics +--- +- kind: + UseOfDotNotNull: ~ + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 10 + fix: + content: + - df.notna + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 10 + parent: ~ + diff --git a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD008_fixes.py.snap b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD008_fixes.py.snap new file mode 100644 index 0000000000000..ddcf984442b74 --- /dev/null +++ b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD008_fixes.py.snap @@ -0,0 +1,23 @@ +--- +source: crates/ruff/src/rules/pandas_vet/mod.rs +expression: diagnostics +--- +- kind: + UseOfDotAt: ~ + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 5 + fix: + content: + - df.loc + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 5 + parent: ~ + diff --git a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD009_fixes.py.snap b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD009_fixes.py.snap new file mode 100644 index 0000000000000..c7f0c64b43973 --- /dev/null +++ b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD009_fixes.py.snap @@ -0,0 +1,23 @@ +--- +source: crates/ruff/src/rules/pandas_vet/mod.rs +expression: diagnostics +--- +- kind: + UseOfDotIat: ~ + location: + row: 11 + column: 0 + end_location: + row: 11 + column: 6 + fix: + content: + - df.iloc + location: + row: 11 + column: 0 + end_location: + row: 11 + column: 6 + parent: ~ + diff --git a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD011_fixes.py.snap b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD011_fixes.py.snap new file mode 100644 index 0000000000000..f11df4e9e3d53 --- /dev/null +++ b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD011_fixes.py.snap @@ -0,0 +1,23 @@ +--- +source: crates/ruff/src/rules/pandas_vet/mod.rs +expression: diagnostics +--- +- kind: + UseOfDotValues: ~ + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 9 + fix: + content: + - df.to_numpy() + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 9 + parent: ~ +