✔️ Linting Passed

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

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

Also, I wonder why pytest can't be used here? @glemaitre

From the top of the head, it comes to not request pytest as a required dependency for third-party library that want to use the checks available in estimator_checks.py. Because of this, we want to run some test in test_estimator_checks.py that does not expect pytest to be installed and it seems that in the past, we needed to not have any pytest import in this file.

This is the meaning of the comment on the top of the file:

# We can not use pytest here, because we run
# build_tools/azure/test_pytest_soft_dependency.sh on these
# tests to make sure estimator_checks works without pytest.

@StefanieSenger: I do not know if you have been reviewing Pull Requests, but I think you should be able to review PR for #27090 starting with this Pull Request for instance. :)

This is just a suggestion and you might already focus on something: feel free to review those PRs or not.

Thank you @jjerphan for you trust. Though, I‘m not sure if I can be of use here.

This is the first review that I‘m trying, the changes in the code make sense to me, but I don‘t have anything to add except for a question. This is not really a review, I‘m afraid.

Edit: I've just seen that I'm not done yet. I need some time to understand check_estimator_sparse_data. I cannot work on it for the next 1,5 days, but I will get back to it as soon as I can manage.

So, the idea behind check_estimator_sparse_data, if I understand correctly, is to check whether a bad configured estimator would trigger one of the error messages „doesn't seem to fail gracefully“. When LargeSparseNotSupportedClassifier is tested in test_estimator_checks.py this is supposed to happen (because it claims to accept large sparse data (accept_large_sparse=True), but doesn‘t).

But the check_estimator_sparse_data is only done on sparse matrices, not sparse arrays, and thus the test is. I have no idea if this means something for this PR. (Why is #27090 only concerned with the tests and not with the rest of the codebase?) Should check_estimator_sparse_data be made to iterate through both types?

@StefanieSenger Thanks for noting this issue. I think we should address it in another PR because as you mentioned, we try first to fix the test files.

However, we want the scikit-learn estimators to be accepting sparse arrays and this should be also true for estimator that are compatible with scikit-learn. So we will need to create/duplicate the check_sparse_* from estimator_checks to test for sparse arrays. I assume that we want a mechanism for people to deactivate the test so we would need a new tag. Up to know, I think that "sparse" was never used in the common test. So we could have a "sparse matrix" and "sparse array" for the X_types. I will open a new issue to track this problem.