Skip to content

TST Extend tests for scipy.sparse.*array in sklearn/linear_model/tests/test_passive_aggressive.py#27273

Merged
glemaitre merged 6 commits intoscikit-learn:mainfrom
yuanx749:sparse-test-passive-aggressive
Sep 11, 2023
Merged

TST Extend tests for scipy.sparse.*array in sklearn/linear_model/tests/test_passive_aggressive.py#27273
glemaitre merged 6 commits intoscikit-learn:mainfrom
yuanx749:sparse-test-passive-aggressive

Conversation

@yuanx749
Copy link
Copy Markdown
Contributor

@yuanx749 yuanx749 commented Sep 3, 2023

Towards #27090.

The original test creates X_csr at the module level, so I keep it there. But if parametrization is preferred (to be consistent with other tests), I can revert to a previous commit which uses parametrize.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 3, 2023

✔️ Linting Passed

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

Generated for commit: bc3c051. Link to the linter CI: here

Copy link
Copy Markdown
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @yuanx749 . I think parametrization would be preferred for this series of PRs.

@yuanx749
Copy link
Copy Markdown
Contributor Author

yuanx749 commented Sep 7, 2023

OK, I applied parametrization. @OmarManzoor

Copy link
Copy Markdown
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @yuanx749

@glemaitre glemaitre self-requested a review September 11, 2023 17:58
Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @yuanx749

@glemaitre glemaitre merged commit a0bdf60 into scikit-learn:main Sep 11, 2023
@yuanx749 yuanx749 deleted the sparse-test-passive-aggressive branch September 12, 2023 03:19
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
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