Skip to content
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

[WIP] grid_search: add sample_weight support #2382

Closed
wants to merge 2 commits into from

Conversation

ndawe
Copy link
Member

@ndawe ndawe commented Aug 22, 2013

No description provided.

@jnothman
Copy link
Member

Exciting! But almost anything that changes in grid_search API should also change in cross_val_score. Or rather, they should be refactored as in #2079.

Also: needs tests.

@ndawe
Copy link
Member Author

ndawe commented Aug 22, 2013

Thanks for the quick review @jnothman! I will implement the changes in cross_val_score and add some tests.

@jnothman
Copy link
Member

It wasn't a full review yet, I'll admit, though I'm excited for the
feature. I'll have time for that in a couple of weeks. I really would like
to see the parallel code deduplicated though.

On Fri, Aug 23, 2013 at 5:47 AM, Noel Dawe notifications@github.com wrote:

Thanks for the quick review @jnothman https://github.com/jnothman! I
will implement the changes in cross_val_score and add some tests.


Reply to this email directly or view it on GitHubhttps://github.com//pull/2382#issuecomment-23119508
.

@ndawe
Copy link
Member Author

ndawe commented Aug 22, 2013

@jnothman ok, maybe I can ping someone else for a review in the meantime after implementing your suggestions.

@ndawe
Copy link
Member Author

ndawe commented Aug 23, 2013

I am going to merge this PR with #1574 since I need those weighted score metrics for the unit tests.

@ndawe
Copy link
Member Author

ndawe commented Aug 23, 2013

Continuing in #1574 where grid search unit tests have been implemented.

@ndawe ndawe closed this Aug 23, 2013
@ndawe ndawe deleted the grid_search_sample_weight branch August 23, 2013 09:02
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.

2 participants