Skip to content

ENH add score_samples method in base search CV and add tests#17478

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 9 commits into from
Jun 8, 2020

Conversation

teonbrooks
Copy link
Contributor

Reference Issues/PRs

Fixes#12542.
Building on and closes#16275.

What does this implement/fix? Explain your changes.

This adds the ability to call score_samples on the estimator with the best found parameters in *SearchCV instances.

@amueller
Copy link
Member

cc @thomasjpfan who reviewed the previous PR

@teonbrooksteonbrooks changed the title WIP add score_samples method in base search CV and add tests[MRG] add score_samples method in base search CV and add testsJun 6, 2020
@amueller
Copy link
Member

Please fix the linting issues :)

Copy link
Member

@thomasjpfanthomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @teonbrooks !

Copy link
Member

@thomasjpfanthomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our linter runs on the diff made by the PR, so the lint issues outside the diff will not make the linter fail.

Given this was during the sprint, I think this is okay.

Copy link
Member

@thomasjpfanthomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thank you for the PR @teonbrooks !

@thomasjpfan
Copy link
Member

thomasjpfan commented Jun 6, 2020

Please add an entry to the change log at doc/whats_new/v0.24.rst with the Feature tag. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:.

@reshamas
Copy link
Member

#DataUmbrella sprint

fix indenting
Copy link
Member

@thomasjpfanthomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomasjpfanthomasjpfan changed the title [MRG] add score_samples method in base search CV and add testsENH add score_samples method in base search CV and add testsJun 7, 2020
Copy link
Member

@adrinjalaliadrinjalali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM.

@adrinjalaliadrinjalali merged commit 5d04910 into scikit-learn:masterJun 8, 2020
@teonbrooksteonbrooks deleted the tb-search-cv branch June 8, 2020 16:43
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
* add score_samples method in base search CV and add tests * add tests and change implem * fix pep8 * address some pr comments * Update test_search.py remove deprecation warnings * address some linting issues * Update test_search.py * update whats_new fix indenting * update test_search.py Co-authored-by: Mohamed Maskani <maskani.mohamed@gmail.com>
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
* add score_samples method in base search CV and add tests * add tests and change implem * fix pep8 * address some pr comments * Update test_search.py remove deprecation warnings * address some linting issues * Update test_search.py * update whats_new fix indenting * update test_search.py Co-authored-by: Mohamed Maskani <maskani.mohamed@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
@teonbrooks@amueller@thomasjpfan@reshamas@adrinjalali@maskani-moh
close