Skip to content

Commit

Permalink
[numpy] deprecated type aliases (#2810)
Browse files Browse the repository at this point in the history
Closes #2455

Used `NPY` as prefix code as agreed in the issue.
  • Loading branch information
sbrugman committed Feb 14, 2023
1 parent c0eb5c2 commit ac028cd
Show file tree
Hide file tree
Showing 15 changed files with 344 additions and 9 deletions.
7 changes: 7 additions & 0 deletions README.md
Expand Up @@ -162,6 +162,7 @@ This README is also available as [documentation](https://beta.ruff.rs/docs/).
1. [pygrep-hooks (PGH)](#pygrep-hooks-pgh)
1. [Pylint (PL)](#pylint-pl)
1. [tryceratops (TRY)](#tryceratops-try)
1. [NumPy-specific rules (NPY)](#numpy-specific-rules-npy)
1. [Ruff-specific rules (RUF)](#ruff-specific-rules-ruf)<!-- End auto-generated table of contents. -->
1. [Editor Integrations](#editor-integrations)
1. [FAQ](#faq)
Expand Down Expand Up @@ -1487,6 +1488,12 @@ For more, see [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) on PyPI
| TRY301 | raise-within-try | Abstract `raise` to an inner function | |
| TRY400 | error-instead-of-exception | Use `logging.exception` instead of `logging.error` | |

### NumPy-specific rules (NPY)

| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| NPY001 | [numpy-deprecated-type-alias](https://beta.ruff.rs/docs/rules/numpy-deprecated-type-alias/) | Type alias `np.{type_name}` is deprecated, replace with builtin type | 🛠 |

### Ruff-specific rules (RUF)

| Code | Name | Message | Fix |
Expand Down
8 changes: 7 additions & 1 deletion clippy.toml
@@ -1 +1,7 @@
doc-valid-idents = ["StackOverflow", "CodeQL", "IPython", ".."]
doc-valid-idents = [
"StackOverflow",
"CodeQL",
"IPython",
"NumPy",
"..",
]
20 changes: 20 additions & 0 deletions crates/ruff/resources/test/fixtures/numpy/NPY001.py
@@ -0,0 +1,20 @@
import numpy as npy
import numpy as np
import numpy

# Error
npy.bool
npy.int

if dtype == np.object:
...

result = result.select_dtypes([np.byte, np.ubyte, np.short, np.ushort, np.int, np.long])

pdf = pd.DataFrame(
data=[[1, 2, 3]],
columns=["a", "b", "c"],
dtype=numpy.object,
)

_ = arr.astype(np.int)
10 changes: 8 additions & 2 deletions crates/ruff/src/checkers/ast.rs
Expand Up @@ -38,8 +38,8 @@ use crate::rules::{
flake8_django, flake8_errmsg, flake8_implicit_str_concat, flake8_import_conventions,
flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise,
flake8_return, flake8_self, flake8_simplify, flake8_tidy_imports, flake8_type_checking,
flake8_unused_arguments, flake8_use_pathlib, mccabe, pandas_vet, pep8_naming, pycodestyle,
pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
flake8_unused_arguments, flake8_use_pathlib, mccabe, numpy, pandas_vet, pep8_naming,
pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
};
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
Expand Down Expand Up @@ -2145,6 +2145,9 @@ where
if self.settings.rules.enabled(&Rule::TypingTextStrAlias) {
pyupgrade::rules::typing_text_str_alias(self, expr);
}
if self.settings.rules.enabled(&Rule::NumpyDeprecatedTypeAlias) {
numpy::rules::deprecated_type_alias(self, expr);
}

// Ex) List[...]
if !self.in_deferred_string_type_definition
Expand Down Expand Up @@ -2211,6 +2214,9 @@ where
if self.settings.rules.enabled(&Rule::TypingTextStrAlias) {
pyupgrade::rules::typing_text_str_alias(self, expr);
}
if self.settings.rules.enabled(&Rule::NumpyDeprecatedTypeAlias) {
numpy::rules::deprecated_type_alias(self, expr);
}
if self.settings.rules.enabled(&Rule::RewriteMockImport) {
pyupgrade::rules::rewrite_mock_attribute(self, expr);
}
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff/src/codes.rs
Expand Up @@ -587,6 +587,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
// flake8-self
(Flake8Self, "001") => Rule::PrivateMemberAccess,

// numpy
(Numpy, "001") => Rule::NumpyDeprecatedTypeAlias,

// ruff
(Ruff, "001") => Rule::AmbiguousUnicodeCharacterString,
(Ruff, "002") => Rule::AmbiguousUnicodeCharacterDocstring,
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff/src/registry.rs
Expand Up @@ -550,6 +550,8 @@ ruff_macros::register_rules!(
rules::flake8_raise::rules::UnnecessaryParenOnRaiseException,
// flake8-self
rules::flake8_self::rules::PrivateMemberAccess,
// numpy
rules::numpy::rules::NumpyDeprecatedTypeAlias,
// ruff
rules::ruff::rules::AmbiguousUnicodeCharacterString,
rules::ruff::rules::AmbiguousUnicodeCharacterDocstring,
Expand Down Expand Up @@ -695,6 +697,9 @@ pub enum Linter {
/// [tryceratops](https://pypi.org/project/tryceratops/1.1.0/)
#[prefix = "TRY"]
Tryceratops,
/// NumPy-specific rules
#[prefix = "NPY"]
Numpy,
/// Ruff-specific rules
#[prefix = "RUF"]
Ruff,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/mod.rs
Expand Up @@ -33,6 +33,7 @@ pub mod flake8_unused_arguments;
pub mod flake8_use_pathlib;
pub mod isort;
pub mod mccabe;
pub mod numpy;
pub mod pandas_vet;
pub mod pep8_naming;
pub mod pycodestyle;
Expand Down
26 changes: 26 additions & 0 deletions crates/ruff/src/rules/numpy/mod.rs
@@ -0,0 +1,26 @@
//! NumPy-specific rules.
pub(crate) mod rules;

#[cfg(test)]
mod tests {
use std::convert::AsRef;
use std::path::Path;

use anyhow::Result;
use test_case::test_case;

use crate::registry::Rule;
use crate::test::test_path;
use crate::{assert_yaml_snapshot, settings};

#[test_case(Rule::NumpyDeprecatedTypeAlias, Path::new("NPY001.py"); "NPY001")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("numpy").join(path).as_path(),
&settings::Settings::for_rule(rule_code),
)?;
assert_yaml_snapshot!(snapshot, diagnostics);
Ok(())
}
}
87 changes: 87 additions & 0 deletions crates/ruff/src/rules/numpy/rules/deprecated_type_alias.rs
@@ -0,0 +1,87 @@
use ruff_macros::{define_violation, derive_message_formats};
use rustpython_parser::ast::Expr;

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::fix::Fix;
use crate::registry::Diagnostic;
use crate::violation::AlwaysAutofixableViolation;

define_violation!(
/// ## What it does
/// Checks for deprecated NumPy type aliases.
///
/// ## Why is this bad?
/// NumPy's `np.int` has long been an alias of the builtin `int`. The same
/// goes for `np.float`, `np.bool`, and others. These aliases exist
/// primarily primarily for historic reasons, and have been a cause of
/// frequent confusion for newcomers.
///
/// These aliases were been deprecated in 1.20, and removed in 1.24.
///
/// ## Examples
/// ```python
/// import numpy as np
///
/// np.bool
/// ```
///
/// Use instead:
/// ```python
/// bool
/// ```
pub struct NumpyDeprecatedTypeAlias {
pub type_name: String,
}
);
impl AlwaysAutofixableViolation for NumpyDeprecatedTypeAlias {
#[derive_message_formats]
fn message(&self) -> String {
let NumpyDeprecatedTypeAlias { type_name } = self;
format!("Type alias `np.{type_name}` is deprecated, replace with builtin type")
}

fn autofix_title(&self) -> String {
let NumpyDeprecatedTypeAlias { type_name } = self;
format!("Replace `np.{type_name}` with builtin type")
}
}

/// NPY001
pub fn deprecated_type_alias(checker: &mut Checker, expr: &Expr) {
if let Some(type_name) = checker.resolve_call_path(expr).and_then(|call_path| {
if call_path.as_slice() == ["numpy", "bool"]
|| call_path.as_slice() == ["numpy", "int"]
|| call_path.as_slice() == ["numpy", "float"]
|| call_path.as_slice() == ["numpy", "complex"]
|| call_path.as_slice() == ["numpy", "object"]
|| call_path.as_slice() == ["numpy", "str"]
|| call_path.as_slice() == ["numpy", "long"]
|| call_path.as_slice() == ["numpy", "unicode"]
{
Some(call_path[1])
} else {
None
}
}) {
let mut diagnostic = Diagnostic::new(
NumpyDeprecatedTypeAlias {
type_name: type_name.to_string(),
},
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.amend(Fix::replacement(
match type_name {
"unicode" => "str",
"long" => "int",
_ => type_name,
}
.to_string(),
expr.location,
expr.end_location.unwrap(),
));
}
checker.diagnostics.push(diagnostic);
}
}
3 changes: 3 additions & 0 deletions crates/ruff/src/rules/numpy/rules/mod.rs
@@ -0,0 +1,3 @@
pub use deprecated_type_alias::{deprecated_type_alias, NumpyDeprecatedTypeAlias};

mod deprecated_type_alias;
@@ -0,0 +1,138 @@
---
source: crates/ruff/src/rules/numpy/mod.rs
expression: diagnostics
---
- kind:
NumpyDeprecatedTypeAlias:
type_name: bool
location:
row: 6
column: 0
end_location:
row: 6
column: 8
fix:
content:
- bool
location:
row: 6
column: 0
end_location:
row: 6
column: 8
parent: ~
- kind:
NumpyDeprecatedTypeAlias:
type_name: int
location:
row: 7
column: 0
end_location:
row: 7
column: 7
fix:
content:
- int
location:
row: 7
column: 0
end_location:
row: 7
column: 7
parent: ~
- kind:
NumpyDeprecatedTypeAlias:
type_name: object
location:
row: 9
column: 12
end_location:
row: 9
column: 21
fix:
content:
- object
location:
row: 9
column: 12
end_location:
row: 9
column: 21
parent: ~
- kind:
NumpyDeprecatedTypeAlias:
type_name: int
location:
row: 12
column: 71
end_location:
row: 12
column: 77
fix:
content:
- int
location:
row: 12
column: 71
end_location:
row: 12
column: 77
parent: ~
- kind:
NumpyDeprecatedTypeAlias:
type_name: long
location:
row: 12
column: 79
end_location:
row: 12
column: 86
fix:
content:
- int
location:
row: 12
column: 79
end_location:
row: 12
column: 86
parent: ~
- kind:
NumpyDeprecatedTypeAlias:
type_name: object
location:
row: 17
column: 10
end_location:
row: 17
column: 22
fix:
content:
- object
location:
row: 17
column: 10
end_location:
row: 17
column: 22
parent: ~
- kind:
NumpyDeprecatedTypeAlias:
type_name: int
location:
row: 20
column: 15
end_location:
row: 20
column: 21
fix:
content:
- int
location:
row: 20
column: 15
end_location:
row: 20
column: 21
parent: ~

8 changes: 5 additions & 3 deletions crates/ruff_dev/src/generate_docs.rs
Expand Up @@ -27,9 +27,11 @@ pub fn main(args: &Args) -> Result<()> {
output.push('\n');

let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap();
output.push_str(&format!("Derived from the **{}** linter.", linter.name()));
output.push('\n');
output.push('\n');
if linter.url().is_some() {
output.push_str(&format!("Derived from the **{}** linter.", linter.name()));
output.push('\n');
output.push('\n');
}

if let Some(autofix) = rule.autofixable() {
output.push_str(match autofix.available {
Expand Down

0 comments on commit ac028cd

Please sign in to comment.