ruff icon indicating copy to clipboard operation
ruff copied to clipboard

NPY201 rules seem to miss imports for NAN

Open xmatthias opened this issue 1 year ago • 2 comments

Using the following test file should show multiple deprecation warnings - but only shows one while using ruff 0.5.0 (the latest afaik).

from numpy import nan, NaN, NAN

import numpy as np

np.NaN
np.NAN
np.nan

ruff check test_npy.py --preview --select NPY2

test_npy.py:5:1: NPY201 [*] `np.NaN` will be removed in NumPy 2.0. Use `numpy.nan` instead.
  |
3 | import numpy as np
4 | 
5 | np.NaN
  | ^^^^^^ NPY201
6 | np.NAN
7 | np.nan
  |
  = help: Replace with `numpy.nan`

Found 1 error.
[*] 1 fixable with the `--fix` option

In reality, np.NAN is also not valid, so should raise an error, too.

Also, explicit imports (not used through np.xxx) seem to be completely ignored ... Neither is from numpy import NaN, NAN (only from numpy import nan is still valid for numpy2.0).

xmatthias avatar Jul 05 '24 05:07 xmatthias

Thanks for reporting this. I just checked the numpy release notes and they only mention the deprecation of NaN in favor of nan.

Do you have a link to documentation saying that NAN has been deprecated well?

In case you're interested to PR a fix. PR is a good example on how to add new NUMPY deprecations.

MichaReiser avatar Jul 08 '24 08:07 MichaReiser

not official documentation as much - but https://github.com/numpy/numpy/pull/26741 - and the pr actually removing both the NaN, as well as the NAN alias (https://github.com/numpy/numpy/pull/24376/files - search for NAN: Final[float] in numpy/__init__.pyi).

It's not in the release notes - but from actually trying it out - it's clear that it's been removed. My assumption is that it's been forgotten to be mentioned in the release notes - as it's clearly gone - in the same PR as the NaN alias.

xmatthias avatar Jul 08 '24 09:07 xmatthias

Closed by https://github.com/astral-sh/ruff/pull/12292.

charliermarsh avatar Jul 17 '24 02:07 charliermarsh

@charliermarsh i tend to disagree.

As mentioned in the PR, the case of from numpy import nan, NaN, NAN (NAN and NaN shouldn't import as they've been removed) isn't covered in that PR.

Let me know if you'd prefer me to open a new issue for that, though its very closely related to this issue, as i reported both problems together.

xmatthias avatar Jul 17 '24 04:07 xmatthias

@xmatthias -- You're saying that we should also catch from imports for those symbols, rather than just usages? I think that would be a separate issue since it applies to all of the cases in that rule and isn't specific to NAN (unless I'm misunderstanding).

charliermarsh avatar Jul 17 '24 18:07 charliermarsh

yeah essentially exactly that

it's deprecated / removed from numpy just as NaN ... just ruff isn't warning it. You're absolutely correct in that it'd apply to essentially all other entries, too.

I think that would be a separate issue

Works for me - will open one :+1:

xmatthias avatar Jul 17 '24 18:07 xmatthias

Thanks!

charliermarsh avatar Jul 17 '24 18:07 charliermarsh

mmmh, i realize now (while recreating code for the new issue) that while the import doesn't warn - usage does - so never mind :+1:

xmatthias avatar Jul 17 '24 18:07 xmatthias