ENH Array API for `check_consistent_length` #29519

StefanieSenger · 2024-07-18T23:48:15Z

Reference Issues/PRs

towards #26024

What does this implement/fix? Explain your changes.

This PR adds Array API for check_consistent_length.

Some little doc enhancements/typo corrections along the way.

I didn't run the MPS test.

github-actions · 2024-07-18T23:49:28Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 2142635. Link to the linter CI: here}

sklearn/utils/tests/test_validation.py

adrinjalali · 2024-07-19T11:22:21Z

sklearn/utils/tests/test_validation.py


    with pytest.raises(TypeError):
-        check_consistent_length([1, 2], np.array(1))
+        check_consistent_length(xp.asarray([1, 2], device=device), np.array(1))


this wouldn't raise when xp==np, would it?

I have just tried it. I raises with TypeError: Singleton array array(1) cannot be considered a valid collection. from within _num_samples() in sklearn/utils/validation.py:376.

The last four pytest.raises checks are not really testing check_consistent_length(), but some other error mechanism outside of it. It feels not very clean. What would be a good way to handle this? Separate those tests?

StefanieSenger

Thanks for your questions @adrinjalali! I have answered them and maybe we can refactor the tests to make these things more obvious.

sklearn/utils/tests/test_validation.py

StefanieSenger · 2024-07-19T13:44:20Z

sklearn/utils/tests/test_validation.py


    with pytest.raises(TypeError):
-        check_consistent_length([1, 2], np.array(1))
+        check_consistent_length(xp.asarray([1, 2], device=device), np.array(1))


I have just tried it. I raises with TypeError: Singleton array array(1) cannot be considered a valid collection. from within _num_samples() in sklearn/utils/validation.py:376.

The last four pytest.raises checks are not really testing check_consistent_length(), but some other error mechanism outside of it. It feels not very clean. What would be a good way to handle this? Separate those tests?

StefanieSenger · 2024-07-19T13:48:31Z

Also, I don't know why the CI tests "scikit-learn.scikit-learn Expected — Waiting for status to be reported" don't run in my PR?
Edit: now they ran and failed. I will look into it on Monday.

StefanieSenger · 2024-07-22T10:56:45Z

I've fixed the PR, @adrinjalali, though now there is another issue:

FAILED sklearn/metrics/tests/test_common.py::test_array_api_compliance[accuracy_score-check_array_api_multilabel_classification_metric-numpy-None-None] - TypeError: csr_matrix is not a supported array type
FAILED sklearn/metrics/tests/test_common.py::test_array_api_compliance[zero_one_loss-check_array_api_multilabel_classification_metric-numpy-None-None] - TypeError: csr_matrix is not a supported array type

Which comes from within _check_targets that would convert the input arrays into sparse, because they are multilabel. (Also see @EdAbati 's comment in lines 137-140.)

Then, the sparse multilabel y_true and y_pred are input into check_consistent_length within accuracy_score, and the tests fail.

Do I assume right that the PRs that will fix #29452 will also fix this here?

EdAbati · 2024-07-22T11:20:50Z

Hi both :) if you are interested in the discussion around that comment, please see #29336 and #29321 (comment)

ogrisel

Thanks for the PR.

My main suggestion would be to rather keep test_check_consistent_length unchanged to make sure that adding array API support does not introduce any behavioral change: in particular, we still want to test that check_consistent_length can work on Python lists, including nested lists with in-homogeneous lengths such as [[1, 2], [[1, 2]]] for which no equivalent NumPy array exist.

Instead of changing the existing test_check_consistent_length function, I would suggest to add a new test named test_check_consistent_length with the yield_namespace_device_dtype_combinations parametrization to check that calling check_consistent_length on valid array API inputs under the sklearn.config_context(array_api_dispatch=True) context.

Do I assume right that the PRs that will fix #29452 will also fix this here?

Indeed, the TypeError: csr_matrix is not a supported array type failures shall be addressed by #29476 which I am concurrently reviewing.

sklearn/utils/_array_api.py

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

StefanieSenger · 2024-08-01T19:08:35Z

Thank you, @ogrisel! I have refactored the tests as you had suggested (though ruff didn't allow me to have them both have the same function name). Please let me know if there is anything else I should change.

ogrisel

Here is another pass of feedback:

sklearn/utils/tests/test_validation.py

sklearn/utils/validation.py

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

StefanieSenger · 2024-11-27T12:17:29Z

Thanks for your review and the suggestions, @ogrisel.

I have committed what you had suggested and the tests (including the cuda test on Colab) all pass.

ogrisel

Thanks @StefanieSenger! LGTM with small details.

ogrisel · 2024-11-29T08:47:05Z

sklearn/metrics/cluster/_supervised.py

@@ -1184,7 +1184,7 @@ def fowlkes_mallows_score(labels_true, labels_pred, *, sparse=False):

    .. versionadded:: 0.18

-    The Fowlkes-Mallows index (FMI) is defined as the geometric mean between of
+    The Fowlkes-Mallows index (FMI) is defined as the geometric mean of


I agree with the change, but this seems unrelated to the scope of the PR. Maybe next time open small quick dedicated PRs for such quick documentation fixes.

sklearn/utils/tests/test_validation.py

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

lesteve · 2024-12-05T08:48:52Z

Merging, thanks!

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

StefanieSenger added 2 commits July 19, 2024 00:33

first commit

21685e1

add to doc

a5dcdc5

github-actions bot added module:metrics module:utils labels Jul 18, 2024

StefanieSenger commented Jul 18, 2024

View reviewed changes

sklearn/utils/tests/test_validation.py Outdated Show resolved Hide resolved

changelog

ca7f4e9

adrinjalali reviewed Jul 19, 2024

View reviewed changes

StefanieSenger commented Jul 19, 2024

View reviewed changes

StefanieSenger and others added 5 commits July 19, 2024 15:58

delete redundant test inputs

a6917bf

delete accidentylly pushed code

5486f92

fix

89c749a

fix type of lengths

48ba5c5

Merge branch 'main' into array_api_one

c2850de

ogrisel reviewed Jul 26, 2024

View reviewed changes

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

StefanieSenger and others added 3 commits August 1, 2024 20:33

Update sklearn/utils/_array_api.py

1f71479

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

Merge branch 'main' into array_api_one

0828673

refactor tests

cf6581b

ogrisel reviewed Aug 8, 2024

View reviewed changes

sklearn/utils/tests/test_validation.py Outdated Show resolved Hide resolved

sklearn/utils/tests/test_validation.py Show resolved Hide resolved

sklearn/utils/tests/test_validation.py Outdated Show resolved Hide resolved

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

betatim mentioned this pull request Aug 30, 2024

Make more of the "tools" of scikit-learn Array API compatible #26024

Open

rwelsch427 pushed a commit to rwelsch427/scikit-learn that referenced this pull request Aug 31, 2024

Apply PR scikit-learn#29519 as patch

225b25d

rwelsch427 mentioned this pull request Aug 31, 2024

ENH Add Array API compatibility to 'auc' #29755

Open

StefanieSenger and others added 4 commits November 18, 2024 16:20

Merge branch 'main' into array_api_one

61b592a

update changelog

ff516a7

Update sklearn/utils/tests/test_validation.py

6c747eb

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

replace link

5eb699e

StefanieSenger and others added 4 commits November 27, 2024 11:51

changes according to review

98b7350

Merge branch 'main' into array_api_one

90197e5

use python set

5af2feb

add pandas dataframe

53255a2

remove pandas test coupeling

ef8d145

ogrisel approved these changes Nov 29, 2024

View reviewed changes

Update sklearn/utils/tests/test_validation.py

2142635

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

lesteve merged commit a67e833 into scikit-learn:main Dec 5, 2024
30 checks passed

StefanieSenger deleted the array_api_one branch December 5, 2024 09:30

virchan pushed a commit to virchan/scikit-learn that referenced this pull request Dec 9, 2024

ENH Array API for check_consistent_length (scikit-learn#29519)

986874b

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH Array API for `check_consistent_length` #29519

ENH Array API for `check_consistent_length` #29519

StefanieSenger commented Jul 18, 2024

github-actions bot commented Jul 18, 2024 •

edited

Loading

adrinjalali Jul 19, 2024

StefanieSenger Jul 19, 2024 •

edited

Loading

StefanieSenger left a comment

StefanieSenger Jul 19, 2024 •

edited

Loading

StefanieSenger commented Jul 19, 2024 •

edited

Loading

StefanieSenger commented Jul 22, 2024 •

edited

Loading

EdAbati commented Jul 22, 2024

ogrisel left a comment •

edited

Loading

StefanieSenger commented Aug 1, 2024

ogrisel left a comment

StefanieSenger commented Nov 27, 2024

ogrisel left a comment

ogrisel Nov 29, 2024

lesteve commented Dec 5, 2024

ENH Array API for check_consistent_length #29519

ENH Array API for check_consistent_length #29519

Conversation

StefanieSenger commented Jul 18, 2024

Reference Issues/PRs

What does this implement/fix? Explain your changes.

github-actions bot commented Jul 18, 2024 • edited Loading

✔️ Linting Passed

adrinjalali Jul 19, 2024

Choose a reason for hiding this comment

StefanieSenger Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

StefanieSenger left a comment

Choose a reason for hiding this comment

StefanieSenger Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

StefanieSenger commented Jul 19, 2024 • edited Loading

StefanieSenger commented Jul 22, 2024 • edited Loading

EdAbati commented Jul 22, 2024

ogrisel left a comment • edited Loading

Choose a reason for hiding this comment

StefanieSenger commented Aug 1, 2024

ogrisel left a comment

Choose a reason for hiding this comment

StefanieSenger commented Nov 27, 2024

ogrisel left a comment

Choose a reason for hiding this comment

ogrisel Nov 29, 2024

Choose a reason for hiding this comment

lesteve commented Dec 5, 2024

ENH Array API for `check_consistent_length` #29519

ENH Array API for `check_consistent_length` #29519

github-actions bot commented Jul 18, 2024 •

edited

Loading

StefanieSenger Jul 19, 2024 •

edited

Loading

StefanieSenger Jul 19, 2024 •

edited

Loading

StefanieSenger commented Jul 19, 2024 •

edited

Loading

StefanieSenger commented Jul 22, 2024 •

edited

Loading

ogrisel left a comment •

edited

Loading