Skip to content

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

Merged
adrinjalali merged 9 commits intoscikit-learn:masterfrom
teonbrooks:tb-search-cv
Jun 8, 2020
Merged

ENH add score_samples method in base search CV and add tests#17478
adrinjalali merged 9 commits intoscikit-learn:masterfrom
teonbrooks:tb-search-cv

Conversation

@teonbrooks
Copy link
Copy Markdown
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
Copy Markdown
Member

amueller commented Jun 6, 2020

cc @thomasjpfan who reviewed the previous PR

@teonbrooks teonbrooks 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 tests Jun 6, 2020
@amueller
Copy link
Copy Markdown
Member

amueller commented Jun 6, 2020

Please fix the linting issues :)

Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan 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
Copy Markdown
Member

@thomasjpfan thomasjpfan 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
Copy Markdown
Member

@thomasjpfan thomasjpfan 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
Copy Markdown
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
Copy Markdown
Member

reshamas commented Jun 6, 2020

#DataUmbrella sprint

fix indenting
Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasjpfan thomasjpfan changed the title [MRG] add score_samples method in base search CV and add tests ENH add score_samples method in base search CV and add tests Jun 7, 2020
Copy link
Copy Markdown
Member

@adrinjalali adrinjalali 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.

@adrinjalali adrinjalali merged commit 5d04910 into scikit-learn:master Jun 8, 2020
@teonbrooks teonbrooks 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a score_samples method to Pipeline and *SearchCV

6 participants