Skip to content

Commit a6f7100

Browse files
[pycodestyle] Allow dtype comparisons in type-comparison (#9676)
## Summary Per #9570: > `dtype` are a bit of a strange beast, but definitely best thought of as instances, not classes, and they are meant to be comparable not just to their own class, but also to the corresponding scalar types (e.g., `x.dtype == np.float32`) and strings (e.g., `x.dtype == ['i1,i4']`; basically, `__eq__` always tries to do `dtype(other)`. This PR thus allows comparisons to `dtype` in preview. Closes #9570. ## Test Plan `cargo test`
1 parent 50122d2 commit a6f7100

File tree

3 files changed

+46
-0
lines changed

3 files changed

+46
-0
lines changed

crates/ruff_linter/resources/test/fixtures/pycodestyle/E721.py

+12
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,15 @@ def type():
126126
# Okay
127127
if type(value) is str:
128128
...
129+
130+
131+
import numpy as np
132+
133+
#: Okay
134+
x.dtype == float
135+
136+
#: Okay
137+
np.dtype(int) == float
138+
139+
#: E721
140+
dtype == float

crates/ruff_linter/src/rules/pycodestyle/rules/type_comparison.rs

+27
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,14 @@ pub(crate) fn preview_type_comparison(checker: &mut Checker, compare: &ast::Expr
162162
.filter(|(_, op)| matches!(op, CmpOp::Eq | CmpOp::NotEq))
163163
.map(|((left, right), _)| (left, right))
164164
{
165+
// If either expression is a type...
165166
if is_type(left, checker.semantic()) || is_type(right, checker.semantic()) {
167+
// And neither is a `dtype`...
168+
if is_dtype(left, checker.semantic()) || is_dtype(right, checker.semantic()) {
169+
continue;
170+
}
171+
172+
// Disallow the comparison.
166173
checker.diagnostics.push(Diagnostic::new(
167174
TypeComparison {
168175
preview: PreviewMode::Enabled,
@@ -295,3 +302,23 @@ fn is_type(expr: &Expr, semantic: &SemanticModel) -> bool {
295302
_ => false,
296303
}
297304
}
305+
306+
/// Returns `true` if the [`Expr`] appears to be a reference to a NumPy dtype, since:
307+
/// > `dtype` are a bit of a strange beast, but definitely best thought of as instances, not
308+
/// > classes, and they are meant to be comparable not just to their own class, but also to the
309+
/// corresponding scalar types (e.g., `x.dtype == np.float32`) and strings (e.g.,
310+
/// `x.dtype == ['i1,i4']`; basically, __eq__ always tries to do `dtype(other)`).
311+
fn is_dtype(expr: &Expr, semantic: &SemanticModel) -> bool {
312+
match expr {
313+
// Ex) `np.dtype(obj)`
314+
Expr::Call(ast::ExprCall { func, .. }) => semantic
315+
.resolve_call_path(func)
316+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["numpy", "dtype"])),
317+
// Ex) `obj.dtype`
318+
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
319+
// Ex) `obj.dtype`
320+
attr.as_str() == "dtype"
321+
}
322+
_ => false,
323+
}
324+
}

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E721_E721.py.snap

+7
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,11 @@ E721.py:59:4: E721 Use `is` and `is not` for type comparisons, or `isinstance()`
129129
61 | #: Okay
130130
|
131131

132+
E721.py:140:1: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
133+
|
134+
139 | #: E721
135+
140 | dtype == float
136+
| ^^^^^^^^^^^^^^ E721
137+
|
138+
132139

0 commit comments

Comments
 (0)