Ignore `NPY201` inside `except` blocks for compatibility with older numpy versions by AlexWaygood · Pull Request #12490 · astral-sh/ruff

AlexWaygood · 2024-07-24T12:32:45Z

Summary

This PR makes it so that deprecated imports and attribute accesses of numpy members are ignored in the following conditions:

For attribute accesses (e.g. np.ComplexWarning), we only ignore the violation if it's inside an except AttributeError block, and the member is accessed through its non-deprecated name in the associated try block.
For uses of the numpy member where it's simply an ExprName node, we check to see how the numpy member was bound. If it was bound via a from numpy import foo statement, we check to see if that import statement took place inside an except ImportError or except ModuleNotFoundError block. If so, and if the numpy member was imported through its non-deprecated name in the associated try block, we ignore the violation in the same way.

Examples:

import numpy as np

try:
    np.all([True, True])
except AttributError:
    np.alltrue([True, True])  # Okay

try:
    from numpy.exceptions import ComplexWarning
except ImportError:
    from numpy import ComplexWarning

x = ComplexWarning()  # Okay

This was more complex than I expected, since we currently track handled exceptions in the semantic model, but we don't track suspended exceptions. I.e., if we're in a lint rule and we're examining a node inside a try block, we know which exceptions the enclosing StmtTry node handles. But if we're in a lint rule and we're examining a node inside an except block, we don't have that information easily to hand.

Test Plan

Added some fixtures and snapshots.

codspeed-hq · 2024-07-24T12:47:18Z

CodSpeed Performance Report

Merging #12490 will not alter performance

_{Comparing alex/numpy-depr (9c8c7d3) with alex/numpy-depr (bb14c21)}

Summary

✅ 33 untouched benchmarks

github-actions · 2024-07-24T12:55:29Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

MichaReiser · 2024-07-24T13:32:51Z

The lexer benchmark is drunk!

AlexWaygood · 2024-07-24T13:34:39Z

The lexer benchmark is drunk!

Hey, I'll take it!

charliermarsh · 2024-07-24T13:36:01Z

+                return false;
+            };
+            let suspended_exceptions = Exceptions::from_try_stmt(try_node, semantic);
+            if !suspended_exceptions.contains(Exceptions::ATTRIBUTE_ERROR) {


Can we check semantic.handled_exceptions instead? Does that work? Then we wouldn't need to find the try statement or anything.

No, semantic.handled_exceptions records the exceptions if we're visiting the try block of a try/except statement. But here we're visiting the except block of a try/except statement. Hence "suspended_exceptions" rather than "handled_exceptions".

Also we still need to find the node for the try statement so that we can check whether it was accessed via the undeprecated call path in the try block

MichaReiser

Looks good. Maybe give @charliermarsh a grace period of a day to look at the semantic model changes ;)

MichaReiser · 2024-07-24T13:37:48Z

+/// AST visitor that searches an AST tree for [`ast::StmtImportFrom`] nodes
+/// that match a certain [`QualifiedName`].
+struct ImportSearcher<'a> {
+    module: &'a str,
+    name: &'a str,
+    found_import: bool,
+}


I don't think it matters much because we only run this code in the error case, but it is kind of expensive. Do we have no way of resolving the statement in which a binding is declared and then traverse upwards (by using the parent API?)

The situation is that we're visiting an except block in a try/except statement. We find a deprecated numpy access in that except block. We need to find out if there's an undeprecated access in the try block of the try/except statement. The way I do this is by traversing upwards from the node we're linting in the except block (using the parent API) to find the StmtTry node, then from the StmtTry node I then traverse downwards into the try block using the ImportSearcher to find if there are any imports using the undeprecated API.

charliermarsh · 2024-07-24T19:41:13Z


+        if self.semantic.in_exception_handler() {
+            flags |= BindingFlags::IN_EXCEPT_HANDLER;
+        }


Idly wondering if we should just store the semantic model flags on the binding, in theory it's tiny.

As in, get rid of the separate BindingFlags struct altogether, and store the semantic-model flags on Binding instead? Or store the semantic-model flags in addition to the BindingFlags?

Store them in addition (and then remove this flag from BindingFlags). But, it's ok, no need to change.

AlexWaygood added 3 commits July 24, 2024 12:09

Add failing test

67f304c

Ignore deprecated attribute accesses in except AttributeError blocks

1b0137e

Ignore deprecated imports in except ImportError blocks

bc79d9c

AlexWaygood added the rule Implementing or modifying a lint rule label Jul 24, 2024

docs

a2a6aaf

AlexWaygood force-pushed the alex/numpy-depr branch from e3ffb37 to a2a6aaf Compare July 24, 2024 12:40

charliermarsh reviewed Jul 24, 2024

View reviewed changes

MichaReiser approved these changes Jul 24, 2024

View reviewed changes

Optimize

bb14c21

charliermarsh reviewed Jul 24, 2024

View reviewed changes

Comment thread crates/ruff_linter/src/rules/numpy/rules/numpy_2_0_deprecation.rs Outdated

charliermarsh reviewed Jul 24, 2024

View reviewed changes

Comment thread crates/ruff_linter/src/rules/numpy/rules/numpy_2_0_deprecation.rs Outdated

charliermarsh approved these changes Jul 24, 2024

View reviewed changes

Charlie review

9c8c7d3

AlexWaygood enabled auto-merge (squash) July 24, 2024 20:01

AlexWaygood merged commit 928ffd6 into main Jul 24, 2024

AlexWaygood deleted the alex/numpy-depr branch July 24, 2024 20:03

BrewTestBot mentioned this pull request Jul 25, 2024

ruff 0.5.5 Homebrew/homebrew-core#178455

Merged

Conversation

AlexWaygood commented Jul 24, 2024

Summary

Test Plan

Uh oh!

codspeed-hq bot commented Jul 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #12490 will not alter performance

Summary

Uh oh!

github-actions bot commented Jul 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

MichaReiser commented Jul 24, 2024

Uh oh!

AlexWaygood commented Jul 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq bot commented Jul 24, 2024 •

edited

Loading

github-actions bot commented Jul 24, 2024 •

edited

Loading

`ruff-ecosystem` results