Implement NumPy 2.0 migration rule #7702

mtsokol · 2023-09-28T20:58:14Z

Summary

Hi! Currently NumPy Python API is undergoing a cleanup process that will be delivered in NumPy 2.0 (release is planned for the end of the year).
Most changes are rather simple (renaming, removing or moving a member of the main namespace to a new place), and they could be flagged/fixed by an additional ruff rule for numpy (e.g. changing occurrences of np.float_ to np.float64).

Would you accept such rule?

I named it NPY201 in the existing group, so people will receive a heads-up for changes arriving in 2.0 before actually migrating to it.

~~This is still a draft PR.~~ I'm not an expert in rust so if any part of code can be done better please share!

NumPy 2.0 migration guide: https://numpy.org/devdocs/numpy_2_0_migration_guide.html
NEP 52: https://numpy.org/neps/nep-0052-python-api-cleanup.html
NumPy cleanup tracking issue: numpy/numpy#23999

Test Plan

A unit test is provided that checks all rule's fix cases.

charliermarsh · 2023-09-28T22:43:06Z

Happy to include something like this. I need to think a bit more on whether this should be its own rule, or whether we should just extend deprecated_function (and add a deprecated_constant for the remaining rules).

As of what NumPy version are these fixes safe? As in, are there older, supported versions of NumPy for which applying these rules would cause issues? (Ultimately asking: will Ruff need to know the NumPy version?)

mtsokol · 2023-09-28T23:21:29Z

Thank you for the feedback! All these breaking changes will be shipped in 2.0.0 version that is planned for this December. The latest release of numpy is 1.26.0.

Only nightly 2.0.0.dev0 and future 2.0.0 versions are safe to apply these changes.
For past releases these are incompatible ones, therefore we thought to establish it as a separate rule. Downstream users that are still on 1.x would upgrade ruff and get a heads-up errors, which they can suppress by disabling the rule.
Once they decide to upgrade to 2.0 they can run ruff with --fix and get most of changes done (and these renames can be mundane to do manually - I did them for numpy, scipy, etc. and it was magnitude of hundreds of lines of changed code).

If ruff would know numpy version then it could be used to determine whether to apply this rule.

charliermarsh · 2023-10-01T03:18:48Z

Thanks @mtsokol. Sorry if I'm misunderstanding, but some of these fixes would be safe to apply in older versions, right? For example, the rules that suggest using np.inf would be safe to apply, since np.inf exists in older versions, etc.

mtsokol · 2023-10-01T07:55:43Z

@charliermarsh Ah yes! Actually most of changes from the NumPy 2.0 migration guide can be applied to the previous versions. (My last response was confusing - only a few are backward incompatible, e.g. renaming np.int_ to np.long (numpy/numpy#24794) or moving np.byte_bounds to np.lib.array_utils. Right now I'm working on these renames mostly and I forgot to mention that it's just a fraction of all changes).

np.Infinity -> np.inf, np.float_ -> np.float64 etc. are all backward compatible and are safe to apply to previous versions.

mtsokol · 2023-10-02T19:03:44Z

The PR's blueprint is ready from my side - it consist of three types of AST fixes:

AutoImport: the deprecated member of the numpy namespace can be replaced with another, e.g. np.float_ with np.float64 or np.INF with np.inf.
AutoPurePython: the deprecated member can be replaced with python expression, e.g. np.NZERO with -0.0 or np.issubclass_ with issubclass.
Manual: can't be automatically fixed - a guideline is provided on how to fix it.

If the code's shape is correct I will provision the rest of entries and exhaustive unit test. Please share your feedback!

P.S. I get some errors in the CI that I can't reproduce locally:

  |
1 | use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
  |                        ^^^^^^^^^^^ no `AutofixKind` in the root

I followed implementation of other numpy rules. What am I missing?

charliermarsh · 2023-10-02T19:40:17Z

@mtsokol - We recently renamed AutofixKind to AutofixAvailability, so you might be running into something like: GitHub is rebasing your PR on main prior to testing, and so the name doesn't exist anymore.

mtsokol · 2023-10-02T19:56:21Z

@mtsokol - We recently renamed AutofixKind to AutofixAvailability, so you might be running into something like: GitHub is rebasing your PR on main prior to testing, and so the name doesn't exist anymore.

Thank you! Now CI tests are passing.

codspeed-hq · 2023-10-02T19:57:43Z

CodSpeed Performance Report

Merging #7702 will not alter performance

_{Comparing mtsokol:npy201 (b86abcd) with main (f64c389)}

Summary

✅ 25 untouched benchmarks

github-actions · 2023-10-02T20:02:25Z

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+3, -1, 0 error(s))

airflow (+3, -1)

- [*] 16167 fixable with the `--fix` option (6240 hidden fixes can be enabled with the `--unsafe-fixes` option).
+ [*] 16169 fixable with the `--fix` option (6240 hidden fixes can be enabled with the `--unsafe-fixes` option).
+ airflow/serialization/serializers/numpy.py:78:13: NPY201 [*] `np.float_` will be removed in the NumPy 2.0. Use `numpy.float64` instead.
+ airflow/serialization/serializers/numpy.py:78:60: NPY201 [*] `np.complex_` will be removed in the NumPy 2.0. Use `numpy.complex128` instead.

Rules changed: 1

Rule	Changes	Additions	Removals
NPY201	2	2	0

hoxbro · 2023-10-06T20:51:59Z

It looks like you are not changing NaN to nan. Per https://numpy.org/devdocs/numpy_2_0_migration_guide.html

mtsokol · 2023-10-06T21:31:35Z

It looks like you are not changing NaN to nan. Per https://numpy.org/devdocs/numpy_2_0_migration_guide.html

We're still finishing adding new changes but here it's just a matter of adding new entries to match in the rule (I will add the rest soon!). I was curious if the overall design of PR and three types of fixes proposed looks correct.

mtsokol · 2023-10-24T10:36:33Z

Hi @charliermarsh @hoxbro,

We finished Python API refactoring for NumPy 2.0. I updated this PR - now the new rule contains all changes introduced to the main namespace (i.e. members' removals).

Out of all 52 cases in the rule only one is backward incompatible (np.byte_bounds). 51 remaining ones are relevant for both NumPy 1.x and 2.0 releases and contribute to cleaner codebase regardless of NumPy version used. There are 29 cases that can be auto-fixed and 23 that require manual intervention.

I added a unit test that checks all rule's fix cases.

charliermarsh · 2023-10-24T14:09:41Z

Thanks @mtsokol -- will queue this up for review and try to get to it shortly.

charliermarsh · 2023-10-27T15:51:35Z

(Sorry for the delay on this -- we shipped our formatter earlier this week and I've been busy with follow-ups from the release. This is still on my list.)

charliermarsh · 2023-11-03T03:44:20Z

This looks great, thanks @mtsokol! I like the way you structured the rule -- it reads well.

I took some liberties in editing the rule documentation and tweaking a few of the migration messages, with the goal of making them a little more consistent with the copy and voice we use in other rules. If you have any feedback or issues with the changes, just let me know, happy to address in a follow-up PR.

github-actions · 2023-11-03T03:55:01Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+3 -0 violations, +0 -0 fixes in 41 projects)

apache/airflow (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --select ALL --preview

+ airflow/providers/salesforce/hooks/salesforce.py:263:34: NPY201 [*] `np.NaN` will be removed in NumPy 2.0. Use `numpy.nan` instead.
+ airflow/serialization/serializers/numpy.py:78:13: NPY201 [*] `np.float_` will be removed in NumPy 2.0. Use `numpy.float64` instead.
+ airflow/serialization/serializers/numpy.py:78:60: NPY201 [*] `np.complex_` will be removed in NumPy 2.0. Use `numpy.complex128` instead.

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
NPY201	3	3	0	0	0

hoxbro · 2023-11-03T07:53:51Z

Shouldn't the new numpy imports happen at the same level as the existing numpy import?

mtsokol · 2023-11-03T09:10:32Z

@charliermarsh Hi, all tweaks look good to me - thank you for a review!

pllim · 2023-11-03T13:50:44Z

Hello! As a user who does not know much about ruff internals, how do I try this out downstream? Do I only have to modify https://github.com/astropy/astropy/blob/main/.ruff.toml ? If so, how? Is this documented somewhere?

charliermarsh · 2023-11-03T13:55:05Z

@hoxbro - Can you give an example of where this is happening?

charliermarsh · 2023-11-03T13:56:24Z

@pllim -- You shouldn't need to modify your configuration, but you would need to build from source, which just means that you need to have Rust installed. If you have Rust installed, then you should be able to run something like:

cargo run -p ruff_linter -- check ../path/to/astropy --select NPY --preview

hoxbro · 2023-11-03T14:00:35Z

...rc/rules/numpy/snapshots/ruff_linter__rules__numpy__tests__numpy2-deprecation_NPY201.py.snap

+  1 |+from numpy.lib import add_docstring
+1 2 | def func():
+2 3 |     import numpy as np


@charliermarsh, here is an example.

I think that instead we should focus on trying to get this to use np.lib.add_docstring here.

I think this is a great way and will only change the line where changes are needed.

Just needs to be sure that the different submodules are initialized when running import numpy as np.

pllim · 2023-11-03T15:05:23Z

but you would need to build from source

Oh, so this isn't one of those code I can just add to the TOML? Or maybe I can in the future when this feature is released?

Not sure if I want to install rust and build this from source on a CI for a Python package.

hoxbro · 2023-11-03T15:15:23Z

...rc/rules/numpy/snapshots/ruff_linter__rules__numpy__tests__numpy2-deprecation_NPY201.py.snap

+   = help: Use `numpy.lib.array_utils.byte_bounds` instead.
+
+ℹ Fix
+   1  |+from numpy.lib.array_utils import byte_bounds


Sorry to be so noisy after the PR has been merged.

I can't do this import in numpy 1.26.0. It is possible to do from numpy.lib.utils import byte_bounds

Hi! This is the only backward incompatible change in the rule, mentioned in the comment: #7702 (comment). It will be available in array_utils from 2.0.

Then I'm really sorry about the noise 🙂

I should probably make this a suggested fix with a clear message.

Fixed in #8474

charliermarsh · 2023-11-03T15:38:43Z

@pllim -- It'll be available in the next release (it was just merged last night), so later today or early next week.

pllim · 2023-11-03T15:45:18Z

It'll be available in the next release

And after that, I can just edit my .ruff.toml to use this?

zanieb · 2023-11-03T15:46:30Z

@pllim yep!

pllim · 2023-11-03T15:49:39Z

What codes are available? Is there a page where I can look this up? Thanks!!

charliermarsh · 2023-11-03T15:54:07Z

Everything on the rules page is available in the latest release. (We only update the documentation when we release.)

pllim · 2023-11-03T19:09:06Z

Great, thanks! Looking forward to the release.

mtsokol marked this pull request as ready for review October 2, 2023 19:03

mtsokol force-pushed the npy201 branch from 6c09751 to 95d3738 Compare October 2, 2023 19:46

mtsokol force-pushed the npy201 branch 3 times, most recently from 87a96b4 to 6b7cca6 Compare October 24, 2023 09:09

Implement NumPy 2.0 migration rule

8e7bc33

mtsokol force-pushed the npy201 branch from 6b7cca6 to 8e7bc33 Compare October 24, 2023 10:22

charliermarsh added 4 commits November 2, 2023 23:18

Merge branch 'main' into npy201

8abd2bd

Add backticks

478ea69

Move to preview; omit message in some cases

485ac74

Run Clippy

b86abcd

charliermarsh enabled auto-merge (squash) November 3, 2023 03:44

charliermarsh added the rule Implementing or modifying a lint rule label Nov 3, 2023

charliermarsh merged commit d04d964 into astral-sh:main Nov 3, 2023
16 checks passed

mtsokol deleted the npy201 branch November 3, 2023 07:50

hoxbro reviewed Nov 3, 2023

View reviewed changes

miccal mentioned this pull request Nov 3, 2023

ruff 0.1.4 Homebrew/homebrew-core#153286

Merged

ngoldbaum mentioned this pull request Nov 4, 2023

DOC: Migration guide should talk about ruff numpy/numpy#25065

Closed

lucascolley mentioned this pull request Nov 29, 2023

New rule category XP, implement not-array-agnostic-numpy (XP001) #8910

Closed

SigureMo mentioned this pull request Dec 22, 2023

[CodeStyle][ruff] fix E226, NPY201 PaddlePaddle/Paddle#60245

Merged

7 tasks

mtsokol mentioned this pull request Mar 15, 2024

ENH: Should row_stack really be deprecated? numpy/numpy#26032

Open

Implement NumPy 2.0 migration rule #7702

Implement NumPy 2.0 migration rule #7702

Conversation

mtsokol commented Sep 28, 2023 • edited Loading

Summary

Test Plan

charliermarsh commented Sep 28, 2023

mtsokol commented Sep 28, 2023

charliermarsh commented Oct 1, 2023

mtsokol commented Oct 1, 2023 • edited Loading

mtsokol commented Oct 2, 2023 • edited Loading

charliermarsh commented Oct 2, 2023

mtsokol commented Oct 2, 2023

codspeed-hq bot commented Oct 2, 2023 • edited Loading

Merging #7702 will not alter performance

Summary

github-actions bot commented Oct 2, 2023 • edited Loading

PR Check Results

Ecosystem

hoxbro commented Oct 6, 2023

mtsokol commented Oct 6, 2023

mtsokol commented Oct 24, 2023 • edited Loading

charliermarsh commented Oct 24, 2023

charliermarsh commented Oct 27, 2023

charliermarsh commented Nov 3, 2023

github-actions bot commented Nov 3, 2023

ruff-ecosystem results

Linter (stable)

Linter (preview)

hoxbro commented Nov 3, 2023

mtsokol commented Nov 3, 2023

pllim commented Nov 3, 2023

charliermarsh commented Nov 3, 2023

charliermarsh commented Nov 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pllim commented Nov 3, 2023

Choose a reason for hiding this comment

mtsokol Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh commented Nov 3, 2023

pllim commented Nov 3, 2023

zanieb commented Nov 3, 2023

pllim commented Nov 3, 2023

charliermarsh commented Nov 3, 2023

pllim commented Nov 3, 2023

mtsokol commented Sep 28, 2023 •

edited

Loading

mtsokol commented Oct 1, 2023 •

edited

Loading

mtsokol commented Oct 2, 2023 •

edited

Loading

codspeed-hq bot commented Oct 2, 2023 •

edited

Loading

github-actions bot commented Oct 2, 2023 •

edited

Loading

mtsokol commented Oct 24, 2023 •

edited

Loading

`ruff-ecosystem` results

mtsokol Nov 3, 2023 •

edited

Loading