NPY201 triggers on code written to support both numpy 2 and legacy numpy

[jenshnielsen]

Considder an example like this which is explicitly written to support both numpy 2 and legacy numpy

import warnings

try:
    from numpy.exceptions import VisibleDeprecationWarning
except ImportError:
    # numpy < 1.25.0
    from numpy import VisibleDeprecationWarning


with warnings.catch_warnings():
    warnings.filterwarnings(
        "ignore",
        category=VisibleDeprecationWarning,
        message="Creating an ndarray from ragged nested sequences",
    )

Running ruff check file.py with NPY201 enabled triggers

error: Failed to create fix for Numpy2Deprecation: Unable to insert `VisibleDeprecationWarning` into scope due to name conflict
src\qcodes\utils\numpy_utils.py:13:18: NPY201 `np.VisibleDeprecationWarning` will be removed in NumPy 2.0. Use `numpy.exceptions.VisibleDeprecationWarning` instead.
   |
11 |     warnings.filterwarnings(
12 |         "ignore",
13 |         category=VisibleDeprecationWarning,
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^ NPY201
14 |         message="Creating an ndarray from ragged nested sequences",
15 |     )
   |
   = help: Replace with `numpy.exceptions.VisibleDeprecationWarning`

This is a false positive since the deprecated location is only used in legacy numpy versions that do not have a exceptions submodule.

This started happening in 0.5.1 probably as a result of #12065

Keyworkds Numpy, NPY201

Ruff 0.5.1 to 0.5.4

Config:

[tool.ruff.lint]
select = ["NPY201"]

[MichaReiser]

I can see how the violation is undesired in this case, and we could probably specialize the rule to detect this very specific pattern. But I think it's also valid to use a suppression comment in this case to explicitly state that the use of the deprecated API is fine.

@mtsokol what do you think?

[AlexWaygood]

I think this pattern is likely to be quite common for projects that want to write code which is compatible with both versions of numpy. Maintaining compatibility with both versions is likely to be something that many projects are going to want to do for a while, as well.

I'd be okay with not flagging deprecated numpy imports iff:

• The deprecated import is imported in an except ImportError block
• The try block of that except block imports the undeprecated alternative

We should make sure all NPY rules are consistent about this sort of thing, whatever we do, though.

[charliermarsh]

That makes sense to me. We should have code to detect the current set of handled exceptions.

[mtsokol]

I can see how the violation is undesired in this case, and we could probably specialize the rule to detect this very specific pattern. But I think it's also valid to use a suppression comment in this case to explicitly state that the use of the deprecated API is fine.

@mtsokol what do you think?

@MichaReiser I agree that this try catch pattern could be recognized by the rule, or users should add a suppression comment.

One note: Apart from try except pattern there's also an if else used downstream, like here in SciPy:
https://github.com/scipy/scipy/blob/c354a0482e08ef68bcdd2163f9b394f2c8e9a0a5/scipy/_lib/_util.py#L25

But generally try except should be preferred.

[jenshnielsen]

Thanks everybody for the input.

@MichaReiser I agree that this try catch pattern could be recognized by the rule, or users should add a suppression comment.

I personally would be fine with just adding a suppression comment as long as this limitation is documented for the rule.

[AlexWaygood]

I'm working on a fix as per my suggestion in #12476 (comment).