Skip to content

ENH add convergence_warning option to lars_path. #3396

Merged
merged 1 commit into from Jul 16, 2014

6 participants

@amueller
scikit-learn member

Previously plot_sparse_recovery.py had 10k lines output. I don't think you want to know about some of your randomized designs being ill-conditioned.

Also: this was a pain. the class-structure might need some cleaning up ;)

@jnothman
scikit-learn member

What is wrong with using the warnings module to do this (and recommending same to users in the documentation):

warnings.simplefilter('ignore', sklearn.utils.ConvergenceWarning)
@amueller
scikit-learn member

We could, but I find it less pretty. This is also not pretty in the code, but nicer for the user.
I think for the randomized version catching should be the default and I working with warnings is often brittle.
Somehow I wouldn't like to have code in RandomizedLasso that modifies the warning stack.

@agramfort
scikit-learn member

LGTM although you need to fix doctests to make travis happy

@amueller
scikit-learn member

Hum, so depends on if we go with catching the warnings or adding the option.

@GaelVaroquaux
scikit-learn member
@amueller
scikit-learn member

Ok, but would you then also catch them by default in RandomizedLasso or leave them to the user?

@GaelVaroquaux
scikit-learn member
@agramfort
scikit-learn member
@amueller
scikit-learn member

second try.

@GaelVaroquaux
scikit-learn member

👍 for merge. Thanks

@coveralls

Coverage Status

Coverage increased (+0.0%) when pulling d146d23 on amueller:less_loud_randomized_lasso into be5826d on scikit-learn:master.

@ogrisel
scikit-learn member
ogrisel commented Jul 16, 2014

LGTM as well. Merging.

@ogrisel ogrisel merged commit f33e9ac into scikit-learn:master Jul 16, 2014

1 check passed

Details continuous-integration/travis-ci The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.