Skip to content

Added check_X_y to lasso_stability_path()#7534

Merged
agramfort merged 1 commit intoscikit-learn:masterfrom
zorroblue:fix-check
Mar 3, 2017
Merged

Added check_X_y to lasso_stability_path()#7534
agramfort merged 1 commit intoscikit-learn:masterfrom
zorroblue:fix-check

Conversation

@zorroblue
Copy link
Copy Markdown
Contributor

Reference Issue

What does this implement/fix? Explain your changes.

This fixes issue #7405 . I have added a check_X_y() to validate X and y.

Any other comments?

Sparse matrices can be accepted in finding the lasso stability path.

@amueller
Copy link
Copy Markdown
Member

amueller commented Sep 30, 2016

Can you add a test please? Maybe with the NotAnArray class? Though just passing a list is probably enough to catch the error.

@zorroblue
Copy link
Copy Markdown
Contributor Author

zorroblue commented Oct 1, 2016

Yes @amueller
Really sorry for the delay, I will look into the guidelines for the test.
Basically a test that fails for the current code and passes for my commit is to be done right?

@maniteja123
Copy link
Copy Markdown
Contributor

maniteja123 commented Oct 2, 2016

Hi, you are right. The test should check if the change has the intended effect. It is called a non regression test. HTH

@agramfort agramfort merged commit 8c7ab17 into scikit-learn:master Mar 3, 2017
@agramfort
Copy link
Copy Markdown
Member

it's safe enough I guess

@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
herilalaina pushed a commit to herilalaina/scikit-learn that referenced this pull request Mar 26, 2017
massich pushed a commit to massich/scikit-learn that referenced this pull request Apr 26, 2017
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants