Repository: scikit-learn/scikit-learn
Pull request: [MRG+1] Change default error_score to raise_deprecating
#### Reference Issues/PRs
Fixes #10309.

#### What does this implement/fix? Explain your changes.
Changes default parameter of error_score to raise-deprecating

#### Any other comments?
Should be method _fit_and_score(cross_validation.py) be updated in the same way?

Problem statement:
Change the default of *SearchCV's error_score to np.nan
It can be pretty unpleasant when your grid search fails. That's why the `error_score` parameter was introduced. The default `error_score='raise'` helps make absolutely clear that there was a failure by terminating the program, but it might be kind to just set the score to NaN and raise a warning. We could consider deprecating it:
* change the default temporarily to `error_score='raise-deprecating'`
* continue to raise an error with `'raise-deprecating'` but also warn that in the future, this error will not be raised, and the user can choose between `error_score='raise'` or the new default, `error_score=np.nan`.
* change the default to `error_score=np.nan` in a couple of versions time

Other devs may want to give feedback on this idea before we go ahead with implementation/merge.

Hints from issue discussion:
What happens for the refit? Do we just exclude the failed ones and warn about it?
Well, I suppose we'd better be careful when sorting scores to watch out for NaNs (I doubt we check this case atm)... and to raise an error if the `best_estimator_` has a NaN score. We could use `-np.inf` instead so that we get sorting for free.
I'd like to take this on if that's ok. I'm a new contributor and would like to get my first PR in 🙌 
sure, thanks.

is this issue open to be worked on?
Well @oasis789 has not yet opened a PR
@oasis789 any word on this? I can take it over if you're swamped with other stuff
I should be able to do it this weekend. Planned to work on it over Christmas but fell it.
Could I please try to work on this, if it's open to be worked on?

@oasis789, if you have not started work sufficiently, perhaps you should say so and let someone else take it...?

Commit message: Add description of default value of error_score.

Patch to review:
diff --git a/sklearn/model_selection/_search.py b/sklearn/model_selection/_search.py
index 8a4c13c640bc4..9f02b7d95437c 100644
--- a/sklearn/model_selection/_search.py
+++ b/sklearn/model_selection/_search.py
@@ -273,7 +273,7 @@ def __len__(self):
 
 
 def fit_grid_point(X, y, estimator, parameters, train, test, scorer,
-                   verbose, error_score='raise', **fit_params):
+                   verbose, error_score='raise-deprecating', **fit_params):
     """Run fit on one set of parameters.
 
     Parameters
@@ -311,11 +311,12 @@ def fit_grid_point(X, y, estimator, parameters, train, test, scorer,
     **fit_params : kwargs
         Additional parameter passed to the fit function of the estimator.
 
-    error_score : 'raise' (default) or numeric
+    error_score : 'raise' or numeric
         Value to assign to the score if an error occurs in estimator fitting.
         If set to 'raise', the error is raised. If a numeric value is given,
         FitFailedWarning is raised. This parameter does not affect the refit
-        step, which will always raise the error.
+        step, which will always raise the error. From version 0.22 the default
+        will be np.nan
 
     Returns
     -------
@@ -391,7 +392,7 @@ class BaseSearchCV(six.with_metaclass(ABCMeta, BaseEstimator,
     def __init__(self, estimator, scoring=None,
                  fit_params=None, n_jobs=1, iid='warn',
                  refit=True, cv=None, verbose=0, pre_dispatch='2*n_jobs',
-                 error_score='raise', return_train_score=True):
+                 error_score='raise-deprecating', return_train_score=True):
 
         self.scoring = scoring
         self.estimator = estimator
@@ -909,11 +910,12 @@ class GridSearchCV(BaseSearchCV):
     verbose : integer
         Controls the verbosity: the higher, the more messages.
 
-    error_score : 'raise' (default) or numeric
+    error_score : 'raise' or numeric
         Value to assign to the score if an error occurs in estimator fitting.
         If set to 'raise', the error is raised. If a numeric value is given,
         FitFailedWarning is raised. This parameter does not affect the refit
-        step, which will always raise the error.
+        step, which will always raise the error. Default is 'raise' but from
+        version 0.22 it will change to np.nan.
 
     return_train_score : boolean, optional
         If ``False``, the ``cv_results_`` attribute will not include training
@@ -1085,7 +1087,7 @@ class GridSearchCV(BaseSearchCV):
 
     def __init__(self, estimator, param_grid, scoring=None, fit_params=None,
                  n_jobs=1, iid='warn', refit=True, cv=None, verbose=0,
-                 pre_dispatch='2*n_jobs', error_score='raise',
+                 pre_dispatch='2*n_jobs', error_score='raise-deprecating',
                  return_train_score="warn"):
         super(GridSearchCV, self).__init__(
             estimator=estimator, scoring=scoring, fit_params=fit_params,
@@ -1244,11 +1246,12 @@ class RandomizedSearchCV(BaseSearchCV):
         If None, the random number generator is the RandomState instance used
         by `np.random`.
 
-    error_score : 'raise' (default) or numeric
+    error_score : 'raise' or numeric
         Value to assign to the score if an error occurs in estimator fitting.
         If set to 'raise', the error is raised. If a numeric value is given,
         FitFailedWarning is raised. This parameter does not affect the refit
-        step, which will always raise the error.
+        step, which will always raise the error. Default is 'raise' but from
+        version 0.22 it will change to np.nan.
 
     return_train_score : boolean, optional
         If ``False``, the ``cv_results_`` attribute will not include training
@@ -1386,7 +1389,7 @@ class RandomizedSearchCV(BaseSearchCV):
     def __init__(self, estimator, param_distributions, n_iter=10, scoring=None,
                  fit_params=None, n_jobs=1, iid='warn', refit=True, cv=None,
                  verbose=0, pre_dispatch='2*n_jobs', random_state=None,
-                 error_score='raise', return_train_score="warn"):
+                 error_score='raise-deprecating', return_train_score="warn"):
         self.param_distributions = param_distributions
         self.n_iter = n_iter
         self.random_state = random_state
diff --git a/sklearn/model_selection/_validation.py b/sklearn/model_selection/_validation.py
index 6597720ac6a76..45bbd7f4e2041 100644
--- a/sklearn/model_selection/_validation.py
+++ b/sklearn/model_selection/_validation.py
@@ -361,7 +361,7 @@ def _fit_and_score(estimator, X, y, scorer, train, test, verbose,
                    parameters, fit_params, return_train_score=False,
                    return_parameters=False, return_n_test_samples=False,
                    return_times=False, return_estimator=False,
-                   error_score='raise'):
+                   error_score='raise-deprecating'):
     """Fit estimator and compute scores for a given dataset split.
 
     Parameters
@@ -395,11 +395,12 @@ def _fit_and_score(estimator, X, y, scorer, train, test, verbose,
     verbose : integer
         The verbosity level.
 
-    error_score : 'raise' (default) or numeric
+    error_score : 'raise' or numeric
         Value to assign to the score if an error occurs in estimator fitting.
         If set to 'raise', the error is raised. If a numeric value is given,
         FitFailedWarning is raised. This parameter does not affect the refit
-        step, which will always raise the error.
+        step, which will always raise the error. Default is 'raise' but from
+        version 0.22 it will change to np.nan.
 
     parameters : dict or None
         Parameters to be set on the estimator.
@@ -459,7 +460,6 @@ def _fit_and_score(estimator, X, y, scorer, train, test, verbose,
     fit_params = dict([(k, _index_param_value(X, v, train))
                       for k, v in fit_params.items()])
 
-    test_scores = {}
     train_scores = {}
     if parameters is not None:
         estimator.set_params(**parameters)
@@ -484,6 +484,14 @@ def _fit_and_score(estimator, X, y, scorer, train, test, verbose,
         score_time = 0.0
         if error_score == 'raise':
             raise
+        elif error_score == 'raise-deprecating':
+            warnings.warn("Estimator failed to fit for these parameters; the "
+                          "error will be raised. If you want a value to be "
+                          "assigned to the score in such cases, then specify "
+                          "error_score parameter. Default is 'raise' but from "
+                          "version 0.22 it will change to np.nan.",
+                          FutureWarning)
+            raise
         elif isinstance(error_score, numbers.Number):
             if is_multimetric:
                 test_scores = dict(zip(scorer.keys(),
diff --git a/sklearn/model_selection/tests/test_validation.py b/sklearn/model_selection/tests/test_validation.py
index 4aab9dadef76a..0ef8d6f50cfbe 100644
--- a/sklearn/model_selection/tests/test_validation.py
+++ b/sklearn/model_selection/tests/test_validation.py
@@ -7,6 +7,7 @@
 import os
 from time import sleep
 
+import pytest
 import numpy as np
 from scipy.sparse import coo_matrix, csr_matrix
 from sklearn.exceptions import FitFailedWarning
@@ -1461,3 +1462,23 @@ def test_fit_and_score():
     # check if the same warning is triggered
     assert_warns_message(FitFailedWarning, warning_message, _fit_and_score,
                          *fit_and_score_args, **fit_and_score_kwargs)
+
+    # check if exception is raised, with default error_score argument
+    assert_raise_message(ValueError, "Failing classifier failed as required",
+                         _fit_and_score, *fit_and_score_args)
+
+    # check if warning was raised, with default error_score argument
+    warning_message = ("Estimator failed to fit for these parameters; the "
+                       "error will be raised. If you want a value to be "
+                       "assigned to the score in such cases, then specify "
+                       "error_score parameter. Default is 'raise' but from "
+                       "version 0.22 it will change to np.nan.")
+    with pytest.raises(ValueError):
+        assert_warns_message(FutureWarning, warning_message, _fit_and_score,
+                             *fit_and_score_args)
+
+    fit_and_score_kwargs = {'error_score': 'raise'}
+    # check if exception was raised, with default error_score='raise'
+    assert_raise_message(ValueError, "Failing classifier failed as required",
+                         _fit_and_score, *fit_and_score_args,
+                         **fit_and_score_kwargs)


Code review comments:
diff_hunk: @@ -484,6 +484,14 @@ def _fit_and_score(estimator, X, y, scorer, train, test, verbose,
         score_time = 0.0
         if error_score == 'raise':
             raise
+        elif error_score == 'raise-deprecating':
+            warnings.warn("Estimator failed to fit for these parameters; the "
+                          "error will be raised. If you want a value to be "
+                          "assigned to the score in such cases, then specify "
+                          "error_score parameter. Default is 'raise' but from "
+                          "version 0.22 it will change to np.nan.",
path: sklearn/model_selection/_validation.py
text: @user2:
I think this warning should be more clear a) about why a user is getting it b) how to make it go away. 
Something along the lines of what @user1 said in https://github.com/scikit-learn/scikit-learn/pull/10677#discussion_r170446753  might be better I think,

>  "From version 0.22, errors during fit will result in a cross validation score of NaN by default. Use error_score='raise' if you want an exception raised."

Once sentence for a) and one for b) . and maybe add  "[...]  or error_score=np.nan to adopt the behavior from version 0.22."