The Wayback Machine - https://web.archive.org/web/20210823163042/https://github.com/scikit-learn/scikit-learn/pull/18343
Skip to content

/ scikit-learn

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

Jump to bottom

FIX allow to output error_score if a scoring failure happen #18343

Merged
merged 36 commits into from Oct 9, 2020
Merged

FIX allow to output error_score if a scoring failure happen #18343

merged 36 commits into from Oct 9, 2020

Conversation

@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Sep 4, 2020

Supersedes and closes #17617
Fixes #17589

Move scoring within try/except to give an error score instead of failing.
dsandeep0138 and others added 28 commits Jun 17, 2020
@dsandeep0138
Set score to error_score when a scorer fails
9059578 
- If a scorer fails during the model selection, set score to
  error_score similar to the fit failing scenario
- Modify scorer fail test cases to expect warning messages
  instead of a ValueError exception
@dsandeep97 @dsandeep0138
Set score to error_score when a scorer fails
ef670ac 
- If a scorer fails during the model selection, set score to
  error_score similar to the fit failing scenario
- Modify scorer fail test cases to expect warning messages
  instead of a ValueError exception
@dsandeep0138
Modify tests to use catch_warnings.
260e1bb 
- Tests are mofified to catch warnings instead of assert_warns
- UserWarning is thrown now when scoring fails, instead of reusing
  the FitFailedWarning which is misleading
@dsandeep0138
Fix lint issues
4f646f9
@dsandeep0138
Fix lint issues#2
2fe7773
@dsandeep0138
Make test case better, identifying the ValueError in error message.
0be3b3e
@dsandeep0138
Update the comment.
d95a90e
@dsandeep0138
Modify all the test cases to use catch_warnings
88ee2d0
@dsandeep0138
Fix some of the remaining lint issues.
4a1be8a
@dsandeep0138
Include more tests to improve code coverage
15aac70
@dsandeep0138
changes to improve patch test coverage.
78b945d
@dsandeep0138
Improvise test cases; Add new test case for score failures.
1f89205
@dsandeep0138
Use pytest.warns instead of warnings.catch_warnings
a89e7f7
@dsandeep0138
Resolve lint issues
c87b706
@dsandeep0138
Simplify test case.
1d1e556
@dsandeep0138
Fix lint issues
449f0de
@dsandeep0138
Update score only for the scorer that fails.
75937fc 
Also handle the case, when MultiMetric scorer fails and throws
an exception.
@dsandeep0138
Minor fix change
6dcad69
@dsandeep0138
Fix lint issues
0d20d26
@dsandeep0138
Make fit function fail if error_score is invalid.
bc269b0
@dsandeep0138
Merge branch 'master' into issue#17589
41aeb35
@dsandeep0138
Update to error_score only when the scorer fails.
be83a8a
@dsandeep0138
Merge branch 'issue#17589' of https://github.com/dsandeep0138/scikit-…
db7f5a1 
…learn into issue#17589
@dsandeep0138
Fix issues with the test case.
c448601
@dsandeep0138
Remove default value of error_score in _score function
d916bbc
@dsandeep0138
Add error_score argument to RFE fit.
908c7fb
@dsandeep0138
Remove error_score from RFE fit.
80e29f6
@glemaitre
Merge remote-tracking branch 'origin/master' into pr/dsandeep0138/17617
4a4c373
@glemaitre glemaitre changed the title Stalled 17617 FIX allow to output error_score if a scoring failure happen Sep 4, 2020
@thomasjpfan
thomasjpfan approved these changes Sep 4, 2020
View changes
Copy link
Member

@thomasjpfan thomasjpfan left a comment

This needs a whats new entry.

Otherwise LGTM.
sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
glemaitre added 6 commits Sep 4, 2020
@glemaitre
iter
7458fa6
@glemaitre
iter
d591b2d
@glemaitre
iter
3cd7e44
@glemaitre
fix import
bfe67b3
@glemaitre
revert warning hidding
bd68a45
@glemaitre
add whats new entry
3cfdc22
@thomasjpfan
thomasjpfan reviewed Sep 4, 2020
View changes
sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_validation.py Show resolved Hide resolved
@glemaitre
iter
a2d4273
@glemaitre glemaitre added this to WAITING FOR REVIEW in Guillaume's pet Sep 7, 2020
@rth rth self-requested a review Sep 23, 2020
@ogrisel
ogrisel approved these changes Oct 8, 2020
View changes
Copy link
Member

@ogrisel ogrisel left a comment

Small API improvement suggestion but otherwise LGTM:
sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved
sklearn/feature_selection/_rfe.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
@glemaitre @ogrisel
Apply suggestions from code review
c24a205 
Co-authored-by: Olivier Grisel <[email protected]>
@ogrisel ogrisel merged commit 7dcb1ac into scikit-learn:master Oct 9, 2020
21 checks passed
21 checks passed
@lgtm-com
LGTM analysis: C/C++ No code changes detected
Details
@lgtm-com
LGTM analysis: JavaScript No code changes detected
Details
@lgtm-com
LGTM analysis: Python No new or fixed alerts
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
@circleci-artifacts-redirector
ci/circleci: doc artifact Link to 0/doc/_changed.html
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
@codecov
codecov/patch 100.00% of diff hit (target 98.16%)
Details
@codecov
codecov/project 98.17% (+0.00%) compared to 5a73abc
Details
@azure-pipelines
scikit-learn.scikit-learn Build #20201008.58 succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linting) Linting succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux py36_conda_openblas) Linux py36_conda_openblas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux py36_ubuntu_atlas) Linux py36_ubuntu_atlas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux pylatest_pip_openblas_pandas) Linux pylatest_pip_openblas_pandas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux32 py36_ubuntu_atlas_32bit) Linux32 py36_ubuntu_atlas_32bit succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux_Runs pylatest_conda_mkl) Linux_Runs pylatest_conda_mkl succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Windows py36_pip_openblas_32bit) Windows py36_pip_openblas_32bit succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Windows py37_conda_mkl) Windows py37_conda_mkl succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (macOS pylatest_conda_mkl) macOS pylatest_conda_mkl succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (macOS pylatest_conda_mkl_no_openmp) macOS pylatest_conda_mkl_no_openmp succeeded
Details
@ogrisel
Copy link
Member

@ogrisel ogrisel commented Oct 9, 2020

Thanks @dsandeep0138 and @glemaitre!
amrcode added a commit to amrcode/scikit-learn that referenced this pull request Oct 19, 2020
@glemaitre @dsandeep97
@ogrisel @amrcode
FIX allow to output error_score if a scoring failure happen (scikit-…
e2ed261 
…learn#18343)

Co-authored-by: Devi Sandeep Endluri <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
jayzed82 added a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
@glemaitre @dsandeep97
@ogrisel @jayzed82
FIX allow to output error_score if a scoring failure happen (scikit-…
666325d 
…learn#18343)

Co-authored-by: Devi Sandeep Endluri <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
@glemaitre glemaitre moved this from WAITING FOR REVIEW to WAITING FOR CONSENSUS in Guillaume's pet Jun 30, 2021
@glemaitre glemaitre moved this from WAITING FOR CONSENSUS to TO BE MERGED in Guillaume's pet Jun 30, 2021
@glemaitre glemaitre moved this from TO BE MERGED to MERGED in Guillaume's pet Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan

@ogrisel ogrisel

@rth rth

Assignees
No one assigned
Labels
module:feature_selection module:model_selection Waiting for Reviewer
Projects
Guillaume's pet
MERGED
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

BaseSearchCV _fit_and_score should move scoring into try/except as some scorers can raise an error for particular CV splits
6 participants
@glemaitre @ogrisel @thomasjpfan @cmarmo @dsandeep0138 @dsandeep97