ENH Array API for check_consistent_length

[StefanieSenger]

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]

✔️ Linting Passed

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

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

[adrinjalali]

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

[StefanieSenger]

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.

[StefanieSenger]

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]

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]

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]

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.

[StefanieSenger]

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:

[StefanieSenger]

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]

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.

[lesteve]

Merging, thanks!