Skip to content

TST Extend tests for scipy.sparse/*array in sklearn/linear_model/tests/test_coordinate_descent#27226

Merged
glemaitre merged 10 commits intoscikit-learn:mainfrom
Charlie-XIAO:tst_sp_coordinate_descent
Sep 13, 2023
Merged

TST Extend tests for scipy.sparse/*array in sklearn/linear_model/tests/test_coordinate_descent#27226
glemaitre merged 10 commits intoscikit-learn:mainfrom
Charlie-XIAO:tst_sp_coordinate_descent

Conversation

@Charlie-XIAO
Copy link
Copy Markdown
Contributor

@Charlie-XIAO Charlie-XIAO commented Aug 30, 2023

Towards #27090.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 30, 2023 •
edited
Loading

✔️ Linting Passed

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

Generated for commit: 44452f9. Link to the linter CI: here

@jjerphan jjerphan mentioned this pull request Aug 31, 2023
Closed
OmarManzoor
OmarManzoor previously approved these changes Sep 1, 2023
View reviewed changes
@OmarManzoor OmarManzoor self-requested a review September 1, 2023 09:40
@OmarManzoor OmarManzoor dismissed their stale review September 1, 2023 09:42

Did not check the CI errors

OmarManzoor
OmarManzoor reviewed Sep 1, 2023
View reviewed changes
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 @Charlie-XIAO. Please see the comment in regards to fixing the CI error.

@Charlie-XIAO
Copy link
Copy Markdown
Contributor Author

Charlie-XIAO commented Sep 1, 2023 •
edited
Loading

I think I'm messing something up. This I should be fixing test_coordinate_descent but not test_sparse_coordinate_descent and I'm having #27237 for that. I need to check whether I pushed to the wrong branch. Apologies for the confusion.

@Charlie-XIAO
Copy link
Copy Markdown
Contributor Author

Charlie-XIAO commented Sep 1, 2023 •
edited
Loading

I have reverted the changes to the wrong file and made the new changes. I might have been in a hurry and made those mistakes, sorry again for the confusion @OmarManzoor.

@OmarManzoor
Copy link
Copy Markdown
Contributor

OmarManzoor commented Sep 1, 2023

I have reverted the changes to the wrong file and made the new changes. I might have been in a hurry and made those mistakes, sorry again for the confusion @OmarManzoor.

No problem! Thank you for fixing it.

@glemaitre glemaitre self-requested a review September 13, 2023 12:51
glemaitre
glemaitre approved these changes Sep 13, 2023
View reviewed changes
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 @Charlie-XIAO. I assume that you would be OK with this PR @OmarManzoor

OmarManzoor
OmarManzoor approved these changes Sep 13, 2023
View reviewed changes
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

@glemaitre glemaitre enabled auto-merge (squash) September 13, 2023 13:05
@glemaitre glemaitre merged commit d1cff3a into scikit-learn:main Sep 13, 2023
@Charlie-XIAO Charlie-XIAO deleted the tst_sp_coordinate_descent branch September 13, 2023 14:38
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@Charlie-XIAO @glemaitre @REDVM
TST Extend tests for scipy.sparse/*array in `sklearn/linear_model/t…
5dae9b5 
…ests/test_coordinate_descent` (scikit-learn#27226)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glemaitre glemaitre glemaitre approved these changes

@OmarManzoor OmarManzoor OmarManzoor approved these changes

Assignees

No one assigned

Labels

module:linear_model No Changelog Needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Charlie-XIAO @OmarManzoor @glemaitre