MAINT: Add ``array-api-tests`` CI stage, add ``ndarray.__array_namespace__`` by mtsokol · Pull Request #25167 · numpy/numpy

mtsokol · 2023-11-16T21:12:20Z

Hi @rgommers @asmeurer,

This draft PR introduces array-api-tests CI stage. Here are some important notes:

It's the first WIP implementation that I managed to initially run on my fork. I'm sharing it here to start the discussion.
I couldn't find array-api-tests on PyPI, so I followed implementation in https://github.com/google/jax/pull/16099/files, where array-api-tests is cloned, and some issues are manually fixed (for now).
array-api-skips.txt contains and disables quite a lot of tests - the rationale is to merge it first, and then, as Array API PRs arrive, enable tests gradually.
In this PR I added __array_namespace__ to ndarray class and __array_api_version__ to the main namespace.
xp.bool is required to run the tests and it can't be XFAILed, so API: Add and redefine numpy.bool [Array API] #25080 needs to be merged before this PR [EDIT] Merged.

.github/workflows/linux.yml

asmeurer · 2023-11-16T22:38:59Z

array-api-skips.txt

Don't know if there's a cleaner place to put this file than the project root.

asmeurer · 2023-11-16T22:39:26Z

This could also be used to run tests for numpy.array_api, but I guess that's lower priority.

jakevdp · 2023-11-16T22:50:06Z

.github/workflows/linux.yml

+      run: |
+        python -m pip install -r build_requirements.txt
+        python -m pip install -r test_requirements.txt
+        python -m pip install hypothesis!=6.88.4  # 6.88.4 leads to a strange error


This needs to be "hypothesis<6.88.4". The problem is warnings from hypothesis, so it's only an issue is if your pytest configuration errors on warnings. (Reported here: data-apis/array-api-tests#211)

Oh, I see -W below, so you definitely will need this version bound until that is addressed upstream.

This should be fixed by data-apis/array-api-tests#212, so the !=6.88.4 can probably be dropped by updating to a newer array-api-tests commit?

And if not, should it be < rather than !=, because there are newer hypothesis versions out already and the problem is unlikely to only exist in that exact 6.88.4 version?

Actually, hypothesis is already present in test_requirements.txt, so this line does nothing - I think it can be removed.

That's right - removed it.

…ace__

mtsokol · 2023-11-24T13:38:57Z

Hi @rgommers, I rebased the branch and now array_api_tests stage runs and passes (with skips.txt file and ignored warnings) - it's ready for a review.

rgommers

This is looking pretty good to me, and test is green with a clean test log. All my suggestions are pretty small.

rgommers · 2023-11-24T14:16:32Z

array-api-skips.txt

+array_api_tests/test_set_functions.py
+
+# newaxis not included in numpy namespace as of v1.26.2
+array_api_tests/test_constants.py::test_newaxis


This one can be left out I think, since numpy.newaxis exists.

Sure, removed!

rgommers · 2023-11-24T14:20:05Z

.github/workflows/linux.yml

+      run: |
+        python -m pip install -r build_requirements.txt
+        python -m pip install -r test_requirements.txt
+        python -m pip install hypothesis!=6.88.4  # 6.88.4 leads to a strange error


This should be fixed by data-apis/array-api-tests#212, so the !=6.88.4 can probably be dropped by updating to a newer array-api-tests commit?

And if not, should it be < rather than !=, because there are newer hypothesis versions out already and the problem is unlikely to only exist in that exact 6.88.4 version?

rgommers · 2023-11-24T14:21:23Z

array-api-skips.txt

+# newaxis not included in numpy namespace as of v1.26.2
+array_api_tests/test_constants.py::test_newaxis
+
+# linalg.solve issue in numpy.array_api as of v1.26.2 (see numpy#25146)


This issue has been fixed already, and didn't affect the main namespace. Can this be dropped?

Yes, removed.

rgommers · 2023-11-24T14:21:45Z

array-api-skips.txt

+# linalg.solve issue in numpy.array_api as of v1.26.2 (see numpy#25146)
+array_api_tests/test_linalg.py::test_solve
+
+# https://github.com/numpy/numpy/issues/21373


also fixed already

Right, removed.

rgommers · 2023-11-24T14:23:19Z

array-api-skips.txt

+# missing copy arg
+array_api_tests/test_signatures.py::test_func_signature[reshape]
+
+# https://github.com/numpy/numpy/issues/21211


Closed, ultimately fixed by #21218 I believe.

rgommers · 2023-11-24T14:31:25Z

.github/workflows/linux.yml

+      run: |
+        python -m pip install -r build_requirements.txt
+        python -m pip install -r test_requirements.txt
+        python -m pip install hypothesis!=6.88.4  # 6.88.4 leads to a strange error


Actually, hypothesis is already present in test_requirements.txt, so this line does nothing - I think it can be removed.

rgommers · 2023-11-24T14:34:17Z

.github/workflows/linux.yml

+        submodules: 'true'
+        path: 'array-api-tests'
+    - name: Fix array-apis bug
+      # Temporary workaround for https://github.com/data-apis/array-api/issues/631 


This should have been fixed yesterday, can you check if things pass now without this fixup with the latest commit from array-api-tests?

Yes, it's fixed now.

rgommers · 2023-11-24T14:39:04Z

.github/workflows/linux.yml

+      with:
+        submodules: recursive
+        fetch-depth: 0
+    - name: Checkout array-api-tests


This is fine for now, but once a bunch of the updates are made we should instead use a regular git submodule for these tests, so that they get run locally. Right now if this job fails, re-running locally will be a bit of a hassle.

Right - at some point I will make it a submodule.

rgommers · 2023-11-24T14:44:19Z

numpy/_core/src/multiarray/methods.c

+        return NULL;
+    }
+
+    if (strcmp(array_api_version, "2022.12") != 0) {


Calling ndarray.__array_namespace__(api_version='2021.12') should work too. The newer version is a superset of the older one, so we'd want to support both. And when we upgrade to say 2023.12 in the future, asking for 2022.12 should not break. So adding a test to check this will be useful.

Second suggested regression test:

>>> import numpy as np >>> xp = np.arange(2).__array_namespace__() >>> assert xp is np

I added test__array_namespace__ for that purpose.

rgommers · 2023-11-24T14:48:53Z

numpy/_core/src/multiarray/methods.c

+
+    if (strcmp(array_api_version, "2022.12") != 0) {
+        PyErr_Format(PyExc_ValueError,
+                     "Version \"%s\" of Array API is not supported.",


minor:

Suggested change

"Version \"%s\" of Array API is not supported.",

"Version \"%s\" of the Array API Standard is not supported.",

mtsokol · 2023-11-24T20:23:17Z

Ok, I think it's ready on my end.

ngoldbaum · 2023-11-28T19:35:40Z

All looks good to me, merging to unblock the other array API PRs. Thanks @mtsokol!

Just out of curiosity, how is __array_namespace__ supposed to work for a library that only partially supports the array API?

rgommers · 2023-11-28T20:47:41Z

Just out of curiosity, how is __array_namespace__ supposed to work for a library that only partially supports the array API?

It isn't - adding the method is kinda saying "I fully support this". In practice one has to be pragmatic and some gaps that aren't too important can simply be ignored. But if you're missing lots of functionality, don't add this method. Think of it like a C standard - there's defines to check whether say C99 is used, but not "C99 minus x, y and z".

asmeurer · 2023-11-29T03:32:11Z

To add to that, at least the way array-api-compat is currently written, __array_namespace__ will bypass the compat namespace https://github.com/data-apis/array-api-compat/blob/main/array_api_compat/common/_helpers.py#L76-L77. That should probably be fixed though if NumPy is adding it, because we will probably need to maintain some compat wrappers even after @mtsokol's work because of things that are kept for backwards compatibility.

asmeurer · 2023-11-29T03:33:17Z

numpy/_core/src/multiarray/methods.c

+        return NULL;
+    }
+
+    if (strcmp(array_api_version, "2021.12") != 0 &&


Is the point of array_api_version to be forwards compatible, or should this error too?

2022.12 is a superset of 2021.12, so if a user asks for the latter, it's fine to return the former.

rgommers · 2023-11-29T09:03:11Z

To add to that, at least the way array-api-compat is currently written, __array_namespace__ will bypass the compat namespace https://github.com/data-apis/array-api-compat/blob/main/array_api_compat/common/_helpers.py#L76-L77. That should probably be fixed though if NumPy is adding it, because we will probably need to maintain some compat wrappers even after @mtsokol's work because of things that are kept for backwards compatibility.

Hmm, we should get to ~100% compatibility. Do you have specific things in mind? If so, let's open an issue now in array-api-compat and fix it soon?

mtsokol changed the title ~~MAINT: Add array-api-tests CI stage, add ndarray.__array_namespace__~~ MAINT: Add array-api-tests CI stage, add ndarray.__array_namespace__ Nov 16, 2023

github-actions bot added the 03 - Maintenance label Nov 16, 2023

mtsokol self-assigned this Nov 16, 2023

mtsokol added the Numpy 2.0 API Changes label Nov 16, 2023

mtsokol force-pushed the array-api-test-ci branch 2 times, most recently from 0d50cd4 to edd114c Compare November 16, 2023 21:18

mtsokol mentioned this pull request Nov 16, 2023

Tracking issue: Add array API standard support to the main namespace #25076

Closed

23 tasks

asmeurer reviewed Nov 16, 2023

View reviewed changes

.github/workflows/linux.yml Outdated Show resolved Hide resolved

asmeurer reviewed Nov 16, 2023

View reviewed changes

array-api-skips.txt

Copy link

Member

asmeurer Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if there's a cleaner place to put this file than the project root.

jakevdp reviewed Nov 16, 2023

View reviewed changes

mtsokol force-pushed the array-api-test-ci branch from edd114c to f0f337f Compare November 24, 2023 12:59

MAINT: Add array-api-tests CI stage, add numpy.ndarray.__array_namesp…

4373469

…ace__

mtsokol force-pushed the array-api-test-ci branch from f0f337f to 4373469 Compare November 24, 2023 13:15

mtsokol marked this pull request as ready for review November 24, 2023 13:20

rgommers reviewed Nov 24, 2023

View reviewed changes

Apply review comments

724d86e

mtsokol force-pushed the array-api-test-ci branch from 58aaff9 to 724d86e Compare November 24, 2023 19:46

charris changed the title ~~MAINT: Add array-api-tests CI stage, add ndarray.__array_namespace__~~ MAINT: Add array-api-tests CI stage, add ndarray.__array_namespace__ Nov 24, 2023

mtsokol requested a review from rgommers November 27, 2023 17:27

ngoldbaum merged commit 06d7bdf into numpy:main Nov 28, 2023

mtsokol deleted the array-api-test-ci branch November 28, 2023 21:54

asmeurer reviewed Nov 29, 2023

View reviewed changes

Illviljan mentioned this pull request Dec 7, 2023

explicitly skip using __array_namespace__ for numpy.ndarray pydata/xarray#8526

Merged

1 task

mtsokol mentioned this pull request Apr 9, 2024

MAINT: Update array-api-tests job #26237

Merged

charris mentioned this pull request Apr 10, 2024

MAINT: Update array-api-tests job #26256

Merged

mtsokol mentioned this pull request Apr 26, 2024

CI: Add array_api_tests CI job pydata/sparse#668

Merged

	"Version \"%s\" of Array API is not supported.",
	"Version \"%s\" of the Array API Standard is not supported.",

Uh oh!

Comments

Conversation

mtsokol commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmeurer commented Nov 16, 2023

Uh oh!

jakevdp Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakevdp Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

mtsokol commented Nov 24, 2023

Uh oh!

rgommers 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!

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!

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtsokol commented Nov 24, 2023

Uh oh!

ngoldbaum commented Nov 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgommers commented Nov 28, 2023

Uh oh!

asmeurer commented Nov 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

mtsokol commented Nov 16, 2023 •

edited

Loading

jakevdp Nov 16, 2023 •

edited

Loading

jakevdp Nov 16, 2023 •

edited

Loading

ngoldbaum commented Nov 28, 2023 •

edited

Loading