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

Merged
ogrisel merged 9 commits intoscikit-learn:mainfrom
msgomez06:skl_sprint
Aug 23, 2023
Merged

TST Extend tests for scipy.sparse.*array in sklearn/cluster/tests/test_dbscan.py + test_birch.py + test_column_transformer.py#27097
ogrisel merged 9 commits intoscikit-learn:mainfrom
msgomez06:skl_sprint

Conversation

@msgomez06
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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 jjerphan and ogrisel August 22, 2023 10:25
Copy link
Copy Markdown
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