Skip to content

Backport 0.17.X -- FIX in randomized_svd flip sign #6375

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 1 commit into from
Feb 18, 2016

Conversation

jakirkham
Copy link
Contributor

Cherry picks the fix from this commit ( cfa498c ) in master. Also, see the PR ( #6135 ) this was taken from.

Flip sign according to `u` in both cases of `transpose`.
@jakirkham
Copy link
Contributor Author

As this is a trivial cherry pick of a previously passing commit, I am not really sure what the problem is with CircleCI, but it looks like it might be running out of memory after building the documentation. FWIW, Travis CI appears to pass.

@TomDLT
Copy link
Member

TomDLT commented Feb 17, 2016

LGTM

@lesteve
Copy link
Member

lesteve commented Feb 17, 2016

As this is a trivial cherry pick of a previously passing commit, I am not really sure what the problem is with CircleCI.

Looks like the CircleCI problem is due to the fix 3daab8a to fetch_california_housing not having been backported in the 0.17.x branch. cc @ogrisel.

@jakirkham
Copy link
Contributor Author

Hmm...so I tried to cherry-pick that commit ( 3daab8a ), as well, but it did not apply cleanly. As I am not really sure what is going on in that code, I don't feel comfortable resolving those merge conflicts. Would someone else who knows more about it please give it a try?

@lesteve
Copy link
Member

lesteve commented Feb 17, 2016

AFAIK (but I may be wrong about that), backports in maintenance branches are done directly by core contributors and not via PRs.

@jakirkham
Copy link
Contributor Author

No clue. I think this is the first time I have done a PR for this project. So, not sure what the rules are. Though I would assume that most people appreciate having some of their work done for them. :) In any event, I just wanted to make sure this saw its way into a bugfix release as I know it was causing me problems.

@ogrisel
Copy link
Member

ogrisel commented Feb 18, 2016

This looks good to me. I will take care of fixing the documentation in 0.17.X.

ogrisel added a commit that referenced this pull request Feb 18, 2016
Backport 0.17.X -- FIX in randomized_svd flip sign
@ogrisel ogrisel merged commit f679095 into scikit-learn:0.17.X Feb 18, 2016
@jakirkham
Copy link
Contributor Author

Thanks @ogrisel.

@jakirkham jakirkham deleted the backport_fix_svd_sign_flip branch February 18, 2016 14:36
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.

5 participants