mirror of https://github.com/astral-sh/ruff
pandas vet autofix for PD002 and general refactor
This commit is contained in:
parent
63fc912ed8
commit
2ef28f217c
|
|
@ -1086,7 +1086,7 @@ For more, see [flake8-pytest-style](https://pypi.org/project/flake8-pytest-style
|
|||
| ---- | ---- | ------- | --- |
|
||||
| PT001 | incorrect-fixture-parentheses-style | Use `@pytest.fixture{expected_parens}` over `@pytest.fixture{actual_parens}` | 🛠 |
|
||||
| PT002 | fixture-positional-args | Configuration for fixture `{function}` specified via positional args, use kwargs | |
|
||||
| PT003 | extraneous-scope-function | `scope='function'` is implied in `@pytest.fixture()` | |
|
||||
| PT003 | extraneous-scope-function | `scope='function'` is implied in `@pytest.fixture()` | 🛠 |
|
||||
| PT004 | missing-fixture-name-underscore | Fixture `{function}` does not return anything, add leading underscore | |
|
||||
| PT005 | incorrect-fixture-name-underscore | Fixture `{function}` returns a value, remove leading underscore | |
|
||||
| PT006 | parametrize-names-wrong-type | Wrong name(s) type in `@pytest.mark.parametrize`, expected `{expected}` | 🛠 |
|
||||
|
|
@ -1247,7 +1247,7 @@ 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 | |
|
||||
| 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 | |
|
||||
| PD007 | use-of-dot-ix | `.ix` is deprecated; use more explicit `.loc` or `.iloc` | |
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
import pandas as pd
|
||||
|
||||
x = pd.DataFrame()
|
||||
|
||||
x.drop(["a"], axis=1, inplace=True)
|
||||
|
||||
x.drop(["a"], axis=1, inplace=True)
|
||||
|
||||
x.drop(
|
||||
inplace=True,
|
||||
columns=["a"],
|
||||
axis=1,
|
||||
)
|
||||
|
||||
if True:
|
||||
x.drop(
|
||||
inplace=True,
|
||||
columns=["a"],
|
||||
axis=1,
|
||||
)
|
||||
|
|
@ -651,6 +651,17 @@ pub fn to_absolute(relative: Location, base: Location) -> Location {
|
|||
}
|
||||
}
|
||||
|
||||
pub fn to_relative(absolute: Location, base: Location) -> Location {
|
||||
if absolute.row() == base.row() {
|
||||
Location::new(
|
||||
absolute.row() - base.row() + 1,
|
||||
absolute.column() - base.column(),
|
||||
)
|
||||
} else {
|
||||
Location::new(absolute.row() - base.row() + 1, absolute.column())
|
||||
}
|
||||
}
|
||||
|
||||
/// Return `true` if a `Stmt` has leading content.
|
||||
pub fn match_leading_content(stmt: &Stmt, locator: &Locator) -> bool {
|
||||
let range = Range::new(Location::new(stmt.location.row(), 0), stmt.location);
|
||||
|
|
|
|||
|
|
@ -323,11 +323,12 @@ pub fn remove_unused_imports<'a>(
|
|||
}
|
||||
}
|
||||
|
||||
/// Generic function te remove (keyword)arguments in function calls
|
||||
/// and class definitions. (For classes `args` should be considered `bases`)
|
||||
/// Generic function to remove arguments or keyword arguments in function
|
||||
/// calls and class definitions. (For classes `args` should be considered
|
||||
/// `bases`)
|
||||
///
|
||||
/// Supports the removal of parentheses when this is the only (kw)arg left.
|
||||
/// For this behaviour set `remove_parentheses` to `true`.
|
||||
/// For this behavior, set `remove_parentheses` to `true`.
|
||||
pub fn remove_argument(
|
||||
locator: &Locator,
|
||||
stmt_at: Location,
|
||||
|
|
@ -337,18 +338,18 @@ pub fn remove_argument(
|
|||
keywords: &[Keyword],
|
||||
remove_parentheses: bool,
|
||||
) -> Result<Fix> {
|
||||
// TODO: preserve trailing comments
|
||||
// TODO(sbrugman): Preserve trailing comments.
|
||||
let contents = locator.slice_source_code_at(stmt_at);
|
||||
|
||||
let mut fix_start = None;
|
||||
let mut fix_end = None;
|
||||
|
||||
let n_keywords = keywords.len() + args.len();
|
||||
if n_keywords == 0 {
|
||||
let n_arguments = keywords.len() + args.len();
|
||||
if n_arguments == 0 {
|
||||
bail!("No arguments or keywords to remove");
|
||||
}
|
||||
|
||||
if n_keywords == 1 {
|
||||
if n_arguments == 1 {
|
||||
// Case 1: there is only one argument.
|
||||
let mut count: usize = 0;
|
||||
for (start, tok, end) in lexer::make_tokenizer_located(contents, stmt_at).flatten() {
|
||||
|
|
@ -358,7 +359,7 @@ pub fn remove_argument(
|
|||
start
|
||||
} else {
|
||||
Location::new(start.row(), start.column() + 1)
|
||||
})
|
||||
});
|
||||
}
|
||||
count += 1;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ pub fn fix_file(diagnostics: &[Diagnostic], locator: &Locator) -> Option<(String
|
|||
}
|
||||
|
||||
/// Apply a series of fixes.
|
||||
fn apply_fixes<'a>(
|
||||
pub(crate) fn apply_fixes<'a>(
|
||||
fixes: impl Iterator<Item = &'a Fix>,
|
||||
locator: &'a Locator<'a>,
|
||||
) -> (String, usize) {
|
||||
|
|
|
|||
|
|
@ -2468,8 +2468,9 @@ where
|
|||
|
||||
// pandas-vet
|
||||
if self.settings.rules.enabled(&Rule::UseOfInplaceArgument) {
|
||||
self.diagnostics
|
||||
.extend(pandas_vet::rules::inplace_argument(keywords).into_iter());
|
||||
self.diagnostics.extend(
|
||||
pandas_vet::rules::inplace_argument(self, expr, args, keywords).into_iter(),
|
||||
);
|
||||
}
|
||||
pandas_vet::rules::check_call(self, func);
|
||||
|
||||
|
|
|
|||
|
|
@ -369,18 +369,18 @@ ruff_macros::define_rule_mapping!(
|
|||
PGH003 => violations::BlanketTypeIgnore,
|
||||
PGH004 => violations::BlanketNOQA,
|
||||
// pandas-vet
|
||||
PD002 => violations::UseOfInplaceArgument,
|
||||
PD003 => violations::UseOfDotIsNull,
|
||||
PD004 => violations::UseOfDotNotNull,
|
||||
PD007 => violations::UseOfDotIx,
|
||||
PD008 => violations::UseOfDotAt,
|
||||
PD009 => violations::UseOfDotIat,
|
||||
PD010 => violations::UseOfDotPivotOrUnstack,
|
||||
PD011 => violations::UseOfDotValues,
|
||||
PD012 => violations::UseOfDotReadTable,
|
||||
PD013 => violations::UseOfDotStack,
|
||||
PD015 => violations::UseOfPdMerge,
|
||||
PD901 => violations::DfIsABadVariableName,
|
||||
PD002 => rules::pandas_vet::rules::UseOfInplaceArgument,
|
||||
PD003 => rules::pandas_vet::rules::UseOfDotIsNull,
|
||||
PD004 => rules::pandas_vet::rules::UseOfDotNotNull,
|
||||
PD007 => rules::pandas_vet::rules::UseOfDotIx,
|
||||
PD008 => rules::pandas_vet::rules::UseOfDotAt,
|
||||
PD009 => rules::pandas_vet::rules::UseOfDotIat,
|
||||
PD010 => rules::pandas_vet::rules::UseOfDotPivotOrUnstack,
|
||||
PD011 => rules::pandas_vet::rules::UseOfDotValues,
|
||||
PD012 => rules::pandas_vet::rules::UseOfDotReadTable,
|
||||
PD013 => rules::pandas_vet::rules::UseOfDotStack,
|
||||
PD015 => rules::pandas_vet::rules::UseOfPdMerge,
|
||||
PD901 => rules::pandas_vet::rules::DfIsABadVariableName,
|
||||
// flake8-errmsg
|
||||
EM101 => violations::RawStringInException,
|
||||
EM102 => violations::FStringInException,
|
||||
|
|
|
|||
|
|
@ -56,11 +56,15 @@ impl Violation for FixturePositionalArgs {
|
|||
define_violation!(
|
||||
pub struct ExtraneousScopeFunction;
|
||||
);
|
||||
impl Violation for ExtraneousScopeFunction {
|
||||
impl AlwaysAutofixableViolation for ExtraneousScopeFunction {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`scope='function'` is implied in `@pytest.fixture()`")
|
||||
}
|
||||
|
||||
fn autofix_title(&self) -> String {
|
||||
"Remove implied `scope` argument".to_string()
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
use crate::ast::helpers;
|
||||
use rustpython_ast::{Expr, ExprKind, Keyword, Location};
|
||||
|
||||
use crate::ast::types::Range;
|
||||
use crate::autofix::apply_fixes;
|
||||
use crate::autofix::helpers::remove_argument;
|
||||
use crate::fix::Fix;
|
||||
use crate::source_code::Locator;
|
||||
|
||||
fn match_name(expr: &Expr) -> Option<&str> {
|
||||
if let ExprKind::Call { func, .. } = &expr.node {
|
||||
if let ExprKind::Attribute { value, .. } = &func.node {
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
Some(id)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove the `inplace` argument from a function call and replace it with an
|
||||
/// assignment.
|
||||
pub fn fix_inplace_argument(
|
||||
locator: &Locator,
|
||||
expr: &Expr,
|
||||
violation_at: Location,
|
||||
violation_end: Location,
|
||||
args: &[Expr],
|
||||
keywords: &[Keyword],
|
||||
) -> Option<Fix> {
|
||||
if let Ok(fix) = remove_argument(
|
||||
locator,
|
||||
expr.location,
|
||||
violation_at,
|
||||
violation_end,
|
||||
args,
|
||||
keywords,
|
||||
false,
|
||||
) {
|
||||
// Reset the line index.
|
||||
let fix_me = Fix::deletion(
|
||||
helpers::to_relative(fix.location, expr.location),
|
||||
helpers::to_relative(fix.end_location, expr.location),
|
||||
);
|
||||
|
||||
// Apply the deletion step.
|
||||
// TODO(charlie): Find a way to
|
||||
let contents =
|
||||
locator.slice_source_code_range(&Range::new(expr.location, expr.end_location.unwrap()));
|
||||
let (output, _) = apply_fixes([fix_me].iter(), &Locator::new(contents));
|
||||
|
||||
// Obtain the name prefix.
|
||||
let name = match_name(expr)?;
|
||||
|
||||
// Apply the assignment.
|
||||
let new_contents = format!("{name} = {output}");
|
||||
|
||||
// Create the new fix.
|
||||
Some(Fix::replacement(
|
||||
new_contents,
|
||||
expr.location,
|
||||
expr.end_location.unwrap(),
|
||||
))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
//! Rules from [pandas-vet](https://pypi.org/project/pandas-vet/).
|
||||
pub(crate) mod fixes;
|
||||
pub(crate) mod helpers;
|
||||
pub(crate) mod rules;
|
||||
|
||||
|
|
@ -11,11 +12,11 @@ mod tests {
|
|||
use test_case::test_case;
|
||||
use textwrap::dedent;
|
||||
|
||||
use crate::linter::check_path;
|
||||
use crate::linter::{check_path, test_path};
|
||||
use crate::registry::{Rule, RuleCodePrefix};
|
||||
use crate::settings::flags;
|
||||
use crate::source_code::{Indexer, Locator, Stylist};
|
||||
use crate::{directives, rustpython_helpers, settings};
|
||||
use crate::{assert_yaml_snapshot, directives, rustpython_helpers, settings};
|
||||
|
||||
fn rule_code(contents: &str, expected: &[Rule]) -> Result<()> {
|
||||
let contents = dedent(contents);
|
||||
|
|
@ -246,4 +247,17 @@ mod tests {
|
|||
rule_code(code, expected)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(Rule::UseOfInplaceArgument, Path::new("PD002.py"); "PD002")]
|
||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
|
||||
let diagnostics = test_path(
|
||||
Path::new("./resources/test/fixtures/pandas_vet")
|
||||
.join(path)
|
||||
.as_path(),
|
||||
&settings::Settings::for_rule(rule_code),
|
||||
)?;
|
||||
assert_yaml_snapshot!(snapshot, diagnostics);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,165 +0,0 @@
|
|||
use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Located};
|
||||
|
||||
use crate::ast::types::{BindingKind, Range};
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::{Diagnostic, DiagnosticKind, Rule};
|
||||
use crate::violations;
|
||||
|
||||
/// PD002
|
||||
pub fn inplace_argument(keywords: &[Keyword]) -> Option<Diagnostic> {
|
||||
for keyword in keywords {
|
||||
let arg = keyword.node.arg.as_ref()?;
|
||||
|
||||
if arg == "inplace" {
|
||||
let is_true_literal = match &keyword.node.value.node {
|
||||
ExprKind::Constant {
|
||||
value: Constant::Bool(boolean),
|
||||
..
|
||||
} => *boolean,
|
||||
_ => false,
|
||||
};
|
||||
if is_true_literal {
|
||||
return Some(Diagnostic::new(
|
||||
violations::UseOfInplaceArgument,
|
||||
Range::from_located(keyword),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// PD015
|
||||
pub fn use_of_pd_merge(func: &Expr) -> Option<Diagnostic> {
|
||||
if let ExprKind::Attribute { attr, value, .. } = &func.node {
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
if id == "pd" && attr == "merge" {
|
||||
return Some(Diagnostic::new(
|
||||
violations::UseOfPdMerge,
|
||||
Range::from_located(func),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// PD901
|
||||
pub fn assignment_to_df(targets: &[Expr]) -> Option<Diagnostic> {
|
||||
if targets.len() != 1 {
|
||||
return None;
|
||||
}
|
||||
let target = &targets[0];
|
||||
let ExprKind::Name { id, .. } = &target.node else {
|
||||
return None;
|
||||
};
|
||||
if id != "df" {
|
||||
return None;
|
||||
}
|
||||
Some(Diagnostic::new(
|
||||
violations::DfIsABadVariableName,
|
||||
Range::from_located(target),
|
||||
))
|
||||
}
|
||||
|
||||
pub fn check_attr(
|
||||
checker: &mut Checker,
|
||||
attr: &str,
|
||||
value: &Located<ExprKind>,
|
||||
attr_expr: &Located<ExprKind>,
|
||||
) {
|
||||
let rules = &checker.settings.rules;
|
||||
let violation: DiagnosticKind = match attr {
|
||||
"ix" if rules.enabled(&Rule::UseOfDotIx) => violations::UseOfDotIx.into(),
|
||||
"at" if rules.enabled(&Rule::UseOfDotAt) => violations::UseOfDotAt.into(),
|
||||
"iat" if rules.enabled(&Rule::UseOfDotIat) => violations::UseOfDotIat.into(),
|
||||
"values" if rules.enabled(&Rule::UseOfDotValues) => violations::UseOfDotValues.into(),
|
||||
_ => return,
|
||||
};
|
||||
|
||||
// Avoid flagging on function calls (e.g., `df.values()`).
|
||||
if let Some(parent) = checker.current_expr_parent() {
|
||||
if matches!(parent.node, ExprKind::Call { .. }) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
// Avoid flagging on non-DataFrames (e.g., `{"a": 1}.values`).
|
||||
if !super::helpers::is_dataframe_candidate(value) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the target is a named variable, avoid triggering on
|
||||
// irrelevant bindings (like imports).
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
if checker.find_binding(id).map_or(true, |binding| {
|
||||
matches!(
|
||||
binding.kind,
|
||||
BindingKind::Builtin
|
||||
| BindingKind::ClassDefinition
|
||||
| BindingKind::FunctionDefinition
|
||||
| BindingKind::Export(..)
|
||||
| BindingKind::FutureImportation
|
||||
| BindingKind::StarImportation(..)
|
||||
| BindingKind::Importation(..)
|
||||
| BindingKind::FromImportation(..)
|
||||
| BindingKind::SubmoduleImportation(..)
|
||||
)
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(violation, Range::from_located(attr_expr)));
|
||||
}
|
||||
|
||||
pub fn check_call(checker: &mut Checker, func: &Located<ExprKind>) {
|
||||
let rules = &checker.settings.rules;
|
||||
let ExprKind::Attribute { value, attr, .. } = &func.node else {return};
|
||||
let violation: DiagnosticKind = match attr.as_str() {
|
||||
"isnull" if rules.enabled(&Rule::UseOfDotIsNull) => violations::UseOfDotIsNull.into(),
|
||||
"notnull" if rules.enabled(&Rule::UseOfDotNotNull) => violations::UseOfDotNotNull.into(),
|
||||
"pivot" | "unstack" if rules.enabled(&Rule::UseOfDotPivotOrUnstack) => {
|
||||
violations::UseOfDotPivotOrUnstack.into()
|
||||
}
|
||||
"read_table" if rules.enabled(&Rule::UseOfDotReadTable) => {
|
||||
violations::UseOfDotReadTable.into()
|
||||
}
|
||||
"stack" if rules.enabled(&Rule::UseOfDotStack) => violations::UseOfDotStack.into(),
|
||||
_ => return,
|
||||
};
|
||||
|
||||
if !super::helpers::is_dataframe_candidate(value) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the target is a named variable, avoid triggering on
|
||||
// irrelevant bindings (like non-Pandas imports).
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
if checker.find_binding(id).map_or(true, |binding| {
|
||||
if let BindingKind::Importation(.., module) = &binding.kind {
|
||||
module != &"pandas"
|
||||
} else {
|
||||
matches!(
|
||||
binding.kind,
|
||||
BindingKind::Builtin
|
||||
| BindingKind::ClassDefinition
|
||||
| BindingKind::FunctionDefinition
|
||||
| BindingKind::Export(..)
|
||||
| BindingKind::FutureImportation
|
||||
| BindingKind::StarImportation(..)
|
||||
| BindingKind::Importation(..)
|
||||
| BindingKind::FromImportation(..)
|
||||
| BindingKind::SubmoduleImportation(..)
|
||||
)
|
||||
}
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(violation, Range::from_located(func)));
|
||||
}
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
use ruff_macros::derive_message_formats;
|
||||
use rustpython_ast::{Expr, ExprKind};
|
||||
|
||||
use crate::ast::types::Range;
|
||||
use crate::define_violation;
|
||||
use crate::registry::Diagnostic;
|
||||
use crate::violation::Violation;
|
||||
|
||||
define_violation!(
|
||||
pub struct DfIsABadVariableName;
|
||||
);
|
||||
impl Violation for DfIsABadVariableName {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`df` is a bad variable name. Be kinder to your future self.")
|
||||
}
|
||||
}
|
||||
|
||||
/// PD901
|
||||
pub fn assignment_to_df(targets: &[Expr]) -> Option<Diagnostic> {
|
||||
if targets.len() != 1 {
|
||||
return None;
|
||||
}
|
||||
let target = &targets[0];
|
||||
let ExprKind::Name { id, .. } = &target.node else {
|
||||
return None;
|
||||
};
|
||||
if id != "df" {
|
||||
return None;
|
||||
}
|
||||
Some(Diagnostic::new(
|
||||
DfIsABadVariableName,
|
||||
Range::from_located(target),
|
||||
))
|
||||
}
|
||||
|
|
@ -0,0 +1,101 @@
|
|||
use ruff_macros::derive_message_formats;
|
||||
use rustpython_ast::{ExprKind, Located};
|
||||
|
||||
use crate::ast::types::{BindingKind, Range};
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::define_violation;
|
||||
use crate::registry::{Diagnostic, DiagnosticKind, Rule};
|
||||
use crate::rules::pandas_vet::helpers::is_dataframe_candidate;
|
||||
use crate::violation::Violation;
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotIx;
|
||||
);
|
||||
impl Violation for UseOfDotIx {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.ix` is deprecated; use more explicit `.loc` or `.iloc`")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotAt;
|
||||
);
|
||||
impl Violation for UseOfDotAt {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `.loc` instead of `.at`. If speed is important, use numpy.")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotIat;
|
||||
);
|
||||
impl Violation for UseOfDotIat {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `.iloc` instead of `.iat`. If speed is important, use numpy.")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotValues;
|
||||
);
|
||||
impl Violation for UseOfDotValues {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `.to_numpy()` instead of `.values`")
|
||||
}
|
||||
}
|
||||
|
||||
pub fn check_attr(
|
||||
checker: &mut Checker,
|
||||
attr: &str,
|
||||
value: &Located<ExprKind>,
|
||||
attr_expr: &Located<ExprKind>,
|
||||
) {
|
||||
let rules = &checker.settings.rules;
|
||||
let violation: DiagnosticKind = match attr {
|
||||
"ix" if rules.enabled(&Rule::UseOfDotIx) => UseOfDotIx.into(),
|
||||
"at" if rules.enabled(&Rule::UseOfDotAt) => UseOfDotAt.into(),
|
||||
"iat" if rules.enabled(&Rule::UseOfDotIat) => UseOfDotIat.into(),
|
||||
"values" if rules.enabled(&Rule::UseOfDotValues) => UseOfDotValues.into(),
|
||||
_ => return,
|
||||
};
|
||||
|
||||
// Avoid flagging on function calls (e.g., `df.values()`).
|
||||
if let Some(parent) = checker.current_expr_parent() {
|
||||
if matches!(parent.node, ExprKind::Call { .. }) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
// Avoid flagging on non-DataFrames (e.g., `{"a": 1}.values`).
|
||||
if !is_dataframe_candidate(value) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the target is a named variable, avoid triggering on
|
||||
// irrelevant bindings (like imports).
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
if checker.find_binding(id).map_or(true, |binding| {
|
||||
matches!(
|
||||
binding.kind,
|
||||
BindingKind::Builtin
|
||||
| BindingKind::ClassDefinition
|
||||
| BindingKind::FunctionDefinition
|
||||
| BindingKind::Export(..)
|
||||
| BindingKind::FutureImportation
|
||||
| BindingKind::StarImportation(..)
|
||||
| BindingKind::Importation(..)
|
||||
| BindingKind::FromImportation(..)
|
||||
| BindingKind::SubmoduleImportation(..)
|
||||
)
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(violation, Range::from_located(attr_expr)));
|
||||
}
|
||||
|
|
@ -0,0 +1,109 @@
|
|||
use ruff_macros::derive_message_formats;
|
||||
use rustpython_ast::{ExprKind, Located};
|
||||
|
||||
use crate::ast::types::{BindingKind, Range};
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::define_violation;
|
||||
use crate::registry::{Diagnostic, DiagnosticKind, Rule};
|
||||
use crate::rules::pandas_vet::helpers::is_dataframe_candidate;
|
||||
use crate::violation::Violation;
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotIsNull;
|
||||
);
|
||||
impl Violation for UseOfDotIsNull {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.isna` is preferred to `.isnull`; functionality is equivalent")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotNotNull;
|
||||
);
|
||||
impl Violation for UseOfDotNotNull {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.notna` is preferred to `.notnull`; functionality is equivalent")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotPivotOrUnstack;
|
||||
);
|
||||
impl Violation for UseOfDotPivotOrUnstack {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!(
|
||||
"`.pivot_table` is preferred to `.pivot` or `.unstack`; provides same functionality"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotReadTable;
|
||||
);
|
||||
impl Violation for UseOfDotReadTable {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.read_csv` is preferred to `.read_table`; provides same functionality")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotStack;
|
||||
);
|
||||
impl Violation for UseOfDotStack {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.melt` is preferred to `.stack`; provides same functionality")
|
||||
}
|
||||
}
|
||||
|
||||
pub fn check_call(checker: &mut Checker, func: &Located<ExprKind>) {
|
||||
let rules = &checker.settings.rules;
|
||||
let ExprKind::Attribute { value, attr, .. } = &func.node else {return};
|
||||
let violation: DiagnosticKind = match attr.as_str() {
|
||||
"isnull" if rules.enabled(&Rule::UseOfDotIsNull) => UseOfDotIsNull.into(),
|
||||
"notnull" if rules.enabled(&Rule::UseOfDotNotNull) => UseOfDotNotNull.into(),
|
||||
"pivot" | "unstack" if rules.enabled(&Rule::UseOfDotPivotOrUnstack) => {
|
||||
UseOfDotPivotOrUnstack.into()
|
||||
}
|
||||
"read_table" if rules.enabled(&Rule::UseOfDotReadTable) => UseOfDotReadTable.into(),
|
||||
"stack" if rules.enabled(&Rule::UseOfDotStack) => UseOfDotStack.into(),
|
||||
_ => return,
|
||||
};
|
||||
|
||||
if !is_dataframe_candidate(value) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the target is a named variable, avoid triggering on
|
||||
// irrelevant bindings (like non-Pandas imports).
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
if checker.find_binding(id).map_or(true, |binding| {
|
||||
if let BindingKind::Importation(.., module) = &binding.kind {
|
||||
module != &"pandas"
|
||||
} else {
|
||||
matches!(
|
||||
binding.kind,
|
||||
BindingKind::Builtin
|
||||
| BindingKind::ClassDefinition
|
||||
| BindingKind::FunctionDefinition
|
||||
| BindingKind::Export(..)
|
||||
| BindingKind::FutureImportation
|
||||
| BindingKind::StarImportation(..)
|
||||
| BindingKind::Importation(..)
|
||||
| BindingKind::FromImportation(..)
|
||||
| BindingKind::SubmoduleImportation(..)
|
||||
)
|
||||
}
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(violation, Range::from_located(func)));
|
||||
}
|
||||
|
|
@ -0,0 +1,63 @@
|
|||
use ruff_macros::derive_message_formats;
|
||||
use rustpython_ast::{Constant, Expr, ExprKind, Keyword};
|
||||
|
||||
use crate::ast::types::Range;
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::define_violation;
|
||||
use crate::registry::Diagnostic;
|
||||
use crate::rules::pandas_vet::fixes::fix_inplace_argument;
|
||||
use crate::violation::AlwaysAutofixableViolation;
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfInplaceArgument;
|
||||
);
|
||||
impl AlwaysAutofixableViolation for UseOfInplaceArgument {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`inplace=True` should be avoided; it has inconsistent behavior")
|
||||
}
|
||||
|
||||
fn autofix_title(&self) -> String {
|
||||
format!("Assign to variable and remove the `inplace` arg")
|
||||
}
|
||||
}
|
||||
|
||||
/// PD002
|
||||
pub fn inplace_argument(
|
||||
checker: &Checker,
|
||||
expr: &Expr,
|
||||
args: &[Expr],
|
||||
keywords: &[Keyword],
|
||||
) -> Option<Diagnostic> {
|
||||
for keyword in keywords {
|
||||
let arg = keyword.node.arg.as_ref()?;
|
||||
|
||||
if arg == "inplace" {
|
||||
let is_true_literal = match &keyword.node.value.node {
|
||||
ExprKind::Constant {
|
||||
value: Constant::Bool(boolean),
|
||||
..
|
||||
} => *boolean,
|
||||
_ => false,
|
||||
};
|
||||
if is_true_literal {
|
||||
let mut diagnostic =
|
||||
Diagnostic::new(UseOfInplaceArgument, Range::from_located(keyword));
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) = fix_inplace_argument(
|
||||
checker.locator,
|
||||
expr,
|
||||
diagnostic.location,
|
||||
diagnostic.end_location,
|
||||
args,
|
||||
keywords,
|
||||
) {
|
||||
diagnostic.amend(fix);
|
||||
}
|
||||
}
|
||||
return Some(diagnostic);
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
pub use assignment_to_df::{assignment_to_df, DfIsABadVariableName};
|
||||
pub use check_attr::{check_attr, UseOfDotAt, UseOfDotIat, UseOfDotIx, UseOfDotValues};
|
||||
pub use check_call::{
|
||||
check_call, UseOfDotIsNull, UseOfDotNotNull, UseOfDotPivotOrUnstack, UseOfDotReadTable,
|
||||
UseOfDotStack,
|
||||
};
|
||||
pub use inplace_argument::{inplace_argument, UseOfInplaceArgument};
|
||||
pub use pd_merge::{use_of_pd_merge, UseOfPdMerge};
|
||||
|
||||
pub mod assignment_to_df;
|
||||
pub mod check_attr;
|
||||
pub mod check_call;
|
||||
pub mod inplace_argument;
|
||||
pub mod pd_merge;
|
||||
|
|
@ -0,0 +1,32 @@
|
|||
use ruff_macros::derive_message_formats;
|
||||
use rustpython_ast::{Expr, ExprKind};
|
||||
|
||||
use crate::ast::types::Range;
|
||||
use crate::define_violation;
|
||||
use crate::registry::Diagnostic;
|
||||
use crate::violation::Violation;
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfPdMerge;
|
||||
);
|
||||
impl Violation for UseOfPdMerge {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!(
|
||||
"Use `.merge` method instead of `pd.merge` function. They have equivalent \
|
||||
functionality."
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// PD015
|
||||
pub fn use_of_pd_merge(func: &Expr) -> Option<Diagnostic> {
|
||||
if let ExprKind::Attribute { attr, value, .. } = &func.node {
|
||||
if let ExprKind::Name { id, .. } = &value.node {
|
||||
if id == "pd" && attr == "merge" {
|
||||
return Some(Diagnostic::new(UseOfPdMerge, Range::from_located(func)));
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
|
@ -0,0 +1,83 @@
|
|||
---
|
||||
source: src/rules/pandas_vet/mod.rs
|
||||
expression: diagnostics
|
||||
---
|
||||
- kind:
|
||||
UseOfInplaceArgument: ~
|
||||
location:
|
||||
row: 5
|
||||
column: 22
|
||||
end_location:
|
||||
row: 5
|
||||
column: 34
|
||||
fix:
|
||||
content:
|
||||
- "x = x.drop([\"a\"], axis=1)"
|
||||
location:
|
||||
row: 5
|
||||
column: 0
|
||||
end_location:
|
||||
row: 5
|
||||
column: 35
|
||||
parent: ~
|
||||
- kind:
|
||||
UseOfInplaceArgument: ~
|
||||
location:
|
||||
row: 7
|
||||
column: 22
|
||||
end_location:
|
||||
row: 7
|
||||
column: 34
|
||||
fix:
|
||||
content:
|
||||
- "x = x.drop([\"a\"], axis=1)"
|
||||
location:
|
||||
row: 7
|
||||
column: 0
|
||||
end_location:
|
||||
row: 7
|
||||
column: 35
|
||||
parent: ~
|
||||
- kind:
|
||||
UseOfInplaceArgument: ~
|
||||
location:
|
||||
row: 10
|
||||
column: 4
|
||||
end_location:
|
||||
row: 10
|
||||
column: 16
|
||||
fix:
|
||||
content:
|
||||
- x = x.drop(
|
||||
- " columns=[\"a\"],"
|
||||
- " axis=1,"
|
||||
- )
|
||||
location:
|
||||
row: 9
|
||||
column: 0
|
||||
end_location:
|
||||
row: 13
|
||||
column: 1
|
||||
parent: ~
|
||||
- kind:
|
||||
UseOfInplaceArgument: ~
|
||||
location:
|
||||
row: 17
|
||||
column: 8
|
||||
end_location:
|
||||
row: 17
|
||||
column: 20
|
||||
fix:
|
||||
content:
|
||||
- x = x.drop(
|
||||
- " columns=[\"a\"],"
|
||||
- " axis=1,"
|
||||
- " )"
|
||||
location:
|
||||
row: 16
|
||||
column: 4
|
||||
end_location:
|
||||
row: 20
|
||||
column: 5
|
||||
parent: ~
|
||||
|
||||
|
|
@ -4523,133 +4523,6 @@ impl Violation for BlanketNOQA {
|
|||
}
|
||||
}
|
||||
|
||||
// pandas-vet
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfInplaceArgument;
|
||||
);
|
||||
impl Violation for UseOfInplaceArgument {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`inplace=True` should be avoided; it has inconsistent behavior")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotIsNull;
|
||||
);
|
||||
impl Violation for UseOfDotIsNull {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.isna` is preferred to `.isnull`; functionality is equivalent")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotNotNull;
|
||||
);
|
||||
impl Violation for UseOfDotNotNull {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.notna` is preferred to `.notnull`; functionality is equivalent")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotIx;
|
||||
);
|
||||
impl Violation for UseOfDotIx {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.ix` is deprecated; use more explicit `.loc` or `.iloc`")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotAt;
|
||||
);
|
||||
impl Violation for UseOfDotAt {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `.loc` instead of `.at`. If speed is important, use numpy.")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotIat;
|
||||
);
|
||||
impl Violation for UseOfDotIat {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `.iloc` instead of `.iat`. If speed is important, use numpy.")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotPivotOrUnstack;
|
||||
);
|
||||
impl Violation for UseOfDotPivotOrUnstack {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!(
|
||||
"`.pivot_table` is preferred to `.pivot` or `.unstack`; provides same functionality"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotValues;
|
||||
);
|
||||
impl Violation for UseOfDotValues {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `.to_numpy()` instead of `.values`")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotReadTable;
|
||||
);
|
||||
impl Violation for UseOfDotReadTable {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.read_csv` is preferred to `.read_table`; provides same functionality")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfDotStack;
|
||||
);
|
||||
impl Violation for UseOfDotStack {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`.melt` is preferred to `.stack`; provides same functionality")
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct UseOfPdMerge;
|
||||
);
|
||||
impl Violation for UseOfPdMerge {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!(
|
||||
"Use `.merge` method instead of `pd.merge` function. They have equivalent \
|
||||
functionality."
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
define_violation!(
|
||||
pub struct DfIsABadVariableName;
|
||||
);
|
||||
impl Violation for DfIsABadVariableName {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("`df` is a bad variable name. Be kinder to your future self.")
|
||||
}
|
||||
}
|
||||
|
||||
// flake8-errmsg
|
||||
|
||||
define_violation!(
|
||||
|
|
|
|||
Loading…
Reference in New Issue