-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
TST Extend tests for scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ test_birch.py
+ test_column_transformer.py
#27097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo a small fix and a green CI.
Thank you, @msgomez06. 🙂
sklearn/cluster/tests/test_dbscan.py
Outdated
|
||
def test_dbscan_sparse(): | ||
core_sparse, labels_sparse = dbscan(sparse.lil_matrix(X), eps=0.8, min_samples=10) | ||
pytest.mark.parametrize("lil_container", LIL_CONTAINERS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest.mark.parametrize("lil_container", LIL_CONTAINERS) | |
@pytest.mark.parametrize("lil_container", LIL_CONTAINERS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beyond Julien's suggestion we can also avoid introducing redundant parameter combinations as follows:
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ 'test_birch'
This comment was marked as resolved.
This comment was marked as resolved.
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ 'test_birch'scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ test_birch
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ test_birch
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ test_birch
+ test_column_transformer
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ test_birch
+ test_column_transformer
scipy.sparse.*array
in sklearn/cluster/tests/test_dbscan.py
+ test_birch.py
+ test_column_transformer.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well. However to make it easier to review and merge incrementally, please open one PR per submodule next time (unless they are actually dependent on a common change/fix in the code base).
…test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` (scikit-learn#27097)
…test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` (scikit-learn#27097)
…test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` (scikit-learn#27097)
Reference Issues/PRs
Towards 27090.
What does this implement/fix? Explain your changes.
This PR introduces sparse containers' list conditionally to the version of SciPy so that we can extend tests as part of 27090.
Any other comments?