Fix make sure enabling `array_api_dispatch=True` does not break any estimator on NumPy inputs #32846

ogrisel · 2025-12-05T15:22:37Z

This is a follow-up to #32840. This should detect most possible silent breaking in future refactoring of helper functions that are used by both in array API supporting and not supporting code in scikit-learn.

AI usage disclosure

I used AI assistance for:

Mostly bad code suggestions that I had to re-edit manually to get the code working correctly
Test/benchmark generation
Documentation (including examples)
Research and understanding

…imator on NumPy inputs

sklearn/utils/_array_api.py

doc/whats_new/upcoming_changes/array-api/32846.fix.rst

sklearn/utils/_array_api.py

…entation in the memoryviewslices unwrap helper

ogrisel · 2025-12-08T17:14:59Z

cc @lucyleeow @OmarManzoor @betatim @virchan

betatim · 2025-12-09T07:52:51Z

Only just getting to this. I asked @lesteve about the release schedule and he'd like to get 1.8.0 done more or less "now". I think it would be nice to have this PR in, but it will take more than five minutes to review. For me it is ok to send 1.8.0 off without this merged.

I can understand that at some point you need to decide "the 1.8 boat has left, you are too late". Even though it makes me sad to ship it with a bug that we know about. Maybe having a small bug is ok (there are probably more that we haven't yet found) - we will find out if someone tried array API support because they'll report an issue ;)

OmarManzoor

Thank you for the PR @ogrisel

sklearn/utils/_array_api.py

sklearn/utils/estimator_checks.py

ogrisel · 2025-12-09T14:36:34Z

@betatim I think the release-critical bugfixes have already all been merged. This one can indeed wait and this is why I did not milestone it. We can take our time to review carefully, no hurry.

Co-authored-by: Omar Salman <omar.salman2007@gmail.com>

OmarManzoor · 2025-12-09T16:14:03Z

@ogrisel There is a CI failure which seems unrelated to this PR but still seems like something erroneous

OmarManzoor

LGTM. Thank you @ogrisel

ogrisel · 2025-12-10T08:43:51Z

There is a CI failure which seems unrelated to this PR but still seems like something erroneous

Indeed, there is a missing rng seed. Let me open a dedicated PR.

…rs-on-numpy-inputs

lucyleeow

LGTM, thanks for the other fixes here. Nits only

lucyleeow · 2025-12-11T11:08:02Z

sklearn/utils/_array_api.py

        Whether to ignore None objects passed in arrays.

-    remove_types : tuple or list, default=(str,)
+    remove_types : tuple or list, (str, list, tuple)


Suggested change

remove_types : tuple or list, (str, list, tuple)

remove_types : tuple or list, default=(str, list, tuple)

lucyleeow · 2025-12-11T11:12:46Z

sklearn/utils/_array_api.py

        Whether to ignore None objects passed in arrays.

-    remove_types : tuple or list, default=(str,)
+    remove_types : tuple or list, (str, list, tuple)


Suggested change

remove_types : tuple or list, (str, list, tuple)

remove_types : tuple or list, default=(str, list, tuple)

lucyleeow · 2025-12-11T11:14:36Z

sklearn/utils/_array_api.py

    remove_none : bool, default=True
        Whether to ignore None objects passed in arrays.
-    remove_types : tuple or list, default=(str,)
+    remove_types : tuple or list, (str, list, tuple)


Suggested change

remove_types : tuple or list, (str, list, tuple)

remove_types : tuple or list, default=(str, list, tuple)

lucyleeow · 2025-12-12T04:05:12Z

sklearn/utils/tests/test_array_api.py

 @skip_if_array_api_compat_not_configured
-def test_get_namespace_ndarray_with_dispatch():
+@pytest.mark.parametrize("X", [numpy.asarray([1, 2, 3]), [1, 2, 3], (1, 2, 3)])
+def test_get_namespace_ndarray_with_dispatch(X):


nit, to match test above

Suggested change

def test_get_namespace_ndarray_with_dispatch(X):

def test_get_namespace_ndarray_or_similar_default_with_dispatch(X):

lucyleeow · 2025-12-12T04:21:51Z

sklearn/utils/estimator_checks.py

+    When expect_only_array_outputs is False, the check accepts non-array
+    outputs from estimator methods (e.g., sparse data structures). This is


Could you expand on this? I was confused just reading this, but https://github.com/scikit-learn/scikit-learn/pull/32846/changes#r2602981393 cleared it up for me.

lesteve · 2025-12-15T05:24:34Z

sklearn/utils/estimator_checks.py

-            else:
-                assert inverse_result.shape == invese_result_xp_np.shape
-                assert inverse_result.dtype == invese_result_xp_np.dtype
+                assert array_device(inverse_result_xp) == array_device(X_xp)


I think you need a config_context here otherwise this assert statement will always pass, checking that None == None. This was noted some time ago by Stefanie in #31814.

Maybe we should do something about it because it's easy to fall into this trap. One potential idea was to add an assert_same_device that uses array_api_compat.device and does not depend on config_context(array_api_dispatch=True) (and a similar assert function like assert_same_namespace). Also maybe it would be nice to have a better name than device for our config_context-dependent device function since I find the clash with array_api_compat.device a tiny bit confusing.

Suggested change

assert array_device(inverse_result_xp) == array_device(X_xp)

with config_context(array_api_dispatch=True):

assert array_device(result_xp) == array_device(X_xp)

ogrisel added 2 commits December 5, 2025 16:11

Fix make sure enabling array_api_dispatch=True does not break any est…

40c7c82

…imator on NumPy inputs

Better check_array_api_input docstring and parameter name

5f865e1

ogrisel added this to Array API Dec 5, 2025

ogrisel added the Array API label Dec 5, 2025

github-actions bot added module:decomposition module:utils labels Dec 5, 2025

ogrisel commented Dec 5, 2025

View reviewed changes

sklearn/utils/_array_api.py Outdated Show resolved Hide resolved

Add changelog entry

30a67a9

ogrisel added the CUDA CI label Dec 5, 2025

github-actions bot removed the CUDA CI label Dec 5, 2025

ogrisel changed the title ~~Fix make sure enabling array_api_dispatch=True does not break any estimator on NumPy inputs~~ Fix make sure enabling array_api_dispatch=True does not break any estimator on NumPy inputs Dec 5, 2025

Alternative way to handle sliced memory views and list/tuple inputs

bd5b9fb

ogrisel moved this to In Progress in Array API Dec 5, 2025

ogrisel added the CUDA CI label Dec 5, 2025

github-actions bot removed the CUDA CI label Dec 5, 2025

ogrisel commented Dec 5, 2025

View reviewed changes

doc/whats_new/upcoming_changes/array-api/32846.fix.rst Show resolved Hide resolved

ogrisel commented Dec 5, 2025

View reviewed changes

sklearn/utils/_array_api.py Show resolved Hide resolved

ogrisel added the Waiting for Reviewer label Dec 5, 2025

Add inline comment to explain the motivation behind the unsual implem…

7527de4

…entation in the memoryviewslices unwrap helper

OmarManzoor reviewed Dec 9, 2025

View reviewed changes

sklearn/utils/_array_api.py Outdated Show resolved Hide resolved

sklearn/utils/estimator_checks.py Show resolved Hide resolved

ogrisel and others added 2 commits December 9, 2025 15:37

Typo

e16f1f3

Co-authored-by: Omar Salman <omar.salman2007@gmail.com>

More consistent use of expect_only_array_outputs

bb4702b

ogrisel added the CUDA CI label Dec 9, 2025

github-actions bot removed the CUDA CI label Dec 9, 2025

OmarManzoor approved these changes Dec 9, 2025

View reviewed changes

ogrisel mentioned this pull request Dec 10, 2025

FIX missing RNG seeding in test_sag_regressor #32879

Merged

OmarManzoor added Waiting for Second Reviewer First reviewer is done, need a second one! and removed Waiting for Reviewer labels Dec 10, 2025

Merge branch 'main' into array-api-dispatch-should-not-break-estimato…

474f49f

…rs-on-numpy-inputs

adrinjalali assigned ogrisel Dec 11, 2025

lucyleeow approved these changes Dec 12, 2025

View reviewed changes

lesteve reviewed Dec 15, 2025

View reviewed changes

	remove_types : tuple or list, (str, list, tuple)
	remove_types : tuple or list, default=(str, list, tuple)

	def test_get_namespace_ndarray_with_dispatch(X):
	def test_get_namespace_ndarray_or_similar_default_with_dispatch(X):

		When expect_only_array_outputs is False, the check accepts non-array
		outputs from estimator methods (e.g., sparse data structures). This is

	assert array_device(inverse_result_xp) == array_device(X_xp)
	with config_context(array_api_dispatch=True):
	assert array_device(result_xp) == array_device(X_xp)

Uh oh!

Fix make sure enabling array_api_dispatch=True does not break any estimator on NumPy inputs #32846

Are you sure you want to change the base?

Fix make sure enabling array_api_dispatch=True does not break any estimator on NumPy inputs #32846

Conversation

ogrisel commented Dec 5, 2025

AI usage disclosure

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ogrisel commented Dec 8, 2025

Uh oh!

betatim commented Dec 9, 2025

Uh oh!

OmarManzoor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ogrisel commented Dec 9, 2025

Uh oh!

OmarManzoor commented Dec 9, 2025

Uh oh!

OmarManzoor left a comment

Choose a reason for hiding this comment

Uh oh!

ogrisel commented Dec 10, 2025

Uh oh!

lucyleeow left a comment

Choose a reason for hiding this comment

Uh oh!

lucyleeow Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

lucyleeow Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

lucyleeow Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

lucyleeow Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

lucyleeow Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

lesteve Dec 15, 2025 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Fix make sure enabling `array_api_dispatch=True` does not break any estimator on NumPy inputs #32846

Fix make sure enabling `array_api_dispatch=True` does not break any estimator on NumPy inputs #32846

lesteve Dec 15, 2025 •

edited

Loading