8000 TST Extend tests for `scipy.sparse.*array` in `sklearn/cluster/tests/test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` by msgomez06 · Pull Request #27097 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 9 commits into from
Aug 23, 2023

Conversation

msgomez06
Copy link
Contributor

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?

@jjerphan jjerphan changed the title TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py Aug 18, 2023
Copy link
Member
@jjerphan jjerphan left a 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. 🙂


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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)

@github-actions
Copy link
github-actions bot commented Aug 18, 2023

✔️ Linting Passed

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

Generated for commit: 81e3a7b. Link to the linter CI: here

Copy link
Member
@ogrisel ogrisel left a 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:

@msgomez06 msgomez06 changed the title TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + 'test_birch' Aug 21, 2023
@msgomez06

This comment was marked as resolved.

@msgomez06 msgomez06 changed the title TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + 'test_birch' TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + test_birch Aug 21, 2023
@msgomez06 msgomez06 changed the title TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + test_birch TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + test_birch + test_column_transformer Aug 22, 2023
@msgomez06 msgomez06 changed the title TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + test_birch + test_column_transformer TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + test_birch.py + test_column_transformer.py Aug 22, 2023
@msgomez06 msgomez06 requested review from ogrisel and jjerphan August 22, 2023 10:25
Copy link
Member
@ogrisel ogrisel left a 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).

@ogrisel ogrisel merged commit 856fbd0 into scikit-learn:main Aug 23, 2023
@msgomez06 msgomez06 deleted the skl_sprint branch August 23, 2023 18:41
akaashpatelmns pushed a commit to akaashp2000/scikit-learn that referenced this pull request Aug 25, 2023
…test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` (scikit-learn#27097)
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Aug 29, 2023
…test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` (scikit-learn#27097)
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
…test_dbscan.py` + `test_birch.py` + `test_column_transformer.py` (scikit-learn#27097)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0