Skip to content

Conversation

glemaitre
Copy link
Member

@dvro @chkoar If you could check into details the two criteria that I added.

@coveralls
Copy link

coveralls commented Aug 30, 2016

Coverage Status

Coverage increased (+0.007%) to 98.033% when pulling 75e886e on glemaitre:issue_130 into f905274 on scikit-learn-contrib:master.

@glemaitre glemaitre merged commit 75e886e into scikit-learn-contrib:master Aug 31, 2016
@dvro
Copy link
Member

dvro commented Sep 1, 2016

@glemaitre sorry I didn't see it earlier!

It looks fine.

Maybe I'd just change so that after checking case 1, it already returns instead of checking case 2 and case 3; this would avoid an extra O(N) execution of computing the stats for each iteration.

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