Repository: scikit-learn/scikit-learn
Pull request: TST Improve error message in DummyClassifier
Fix #14903 

Add to the error message the constant value passed and the real possible values in the data when the constant value is not in the training data.


Problem statement:
Error could be improved with DummyClassifier constant strategy when constant value not in training data
```py
from sklearn.dummy import DummyClassifier
clf = DummyClassifier(strategy='constant', constant='not-in-dataset')
clf.fit([[1., 2.]], ['class1'])
```

Error:
```
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-6096dbf560dd> in <module>
----> 1 clf.fit([[1., 2.]], ['class1'])

~/miniconda3/lib/python3.7/site-packages/sklearn/dummy.py in fit(self, X, y, sample_weight)
    149             # Checking in case of constant strategy if the constant
    150             # provided by the user is in y.
--> 151             raise ValueError("The constant target value must be "
    152                              "present in training data")
    153 

ValueError: The constant target value must be present in training data
```

We could add in the error message what constant value was provided (in this case `not-in-dataset`) and what the possible values are. This could be something like this (improvement more than welcome):

```
The constant target value must be present in the training data.
You provided: constant='not-in-dataset'. Possible values are: ['class1'].
```

Context: this was seen during the EuroScipy tutorial. The adult census dataset classes has a space in it at the beginning  ` <=50K` and the provided value did not have the space. Putting what the provided value was and what were the possible values would have helped the user fixing the problem.

Hints from issue discussion:
ok, I am looking at this issue
@LakshKD we are having the scikit-learn sprint at EuroScipy. Unless you are here too, would you mind trying to find another issue to work on?

Hopefully you don't mind too much. Basically I think this issue is ideal for someone who is getting started with scikit-learn and I see you are already a scikit-learn contributor.
I'm working on this
> @LakshKD we are having the scikit-learn sprint at EuroScipy. Unless you are here too, would you mind trying to find another issue to work on?
> 
> Hopefully you don't mind too much. Basically I think this issue is ideal for someone who is getting started with scikit-learn and I see you are already a scikit-learn contributor.

ok no problem
Thanks a lot for your understanding @LakshKD!

Commit message: Fix flake8 warning

Patch to review:
diff --git a/sklearn/dummy.py b/sklearn/dummy.py
index ab79321bd4fa3..e8f9bce210015 100644
--- a/sklearn/dummy.py
+++ b/sklearn/dummy.py
@@ -143,13 +143,17 @@ def fit(self, X, y, sample_weight=None):
          self.n_classes_,
          self.class_prior_) = class_distribution(y, sample_weight)
 
-        if (self.strategy == "constant" and
-                any(constant[k] not in self.classes_[k]
-                    for k in range(self.n_outputs_))):
-            # Checking in case of constant strategy if the constant
-            # provided by the user is in y.
-            raise ValueError("The constant target value must be "
-                             "present in training data")
+        if self.strategy == "constant":
+            for k in range(self.n_outputs_):
+                if constant[k] not in self.classes_[k]:
+                    # Checking in case of constant strategy if the constant
+                    # provided by the user is in y.
+                    raise ValueError(
+                        "The constant target value must be present "
+                        "in the training data. "
+                        "You provided constant=%r. "
+                        "Possible values are: %r." % (self.constant,
+                                                      list(self.classes_[k])))
 
         if self.n_outputs_ == 1 and not self.output_2d_:
             self.n_classes_ = self.n_classes_[0]
diff --git a/sklearn/tests/test_dummy.py b/sklearn/tests/test_dummy.py
index 4301a4c07654f..ede5c0dae5053 100644
--- a/sklearn/tests/test_dummy.py
+++ b/sklearn/tests/test_dummy.py
@@ -543,6 +543,11 @@ def test_constant_strategy_exceptions():
                           constant=[2, 0])
     assert_raises(ValueError, clf.fit, X, y)
 
+    clf = DummyClassifier(strategy='constant', constant='not-in-dataset')
+    with pytest.raises(ValueError,
+                       match="constant='not-in-dataset'.+"
+                       "Possible values.+['class1', 'class2']"):
+        clf.fit([[1., 2.], [2., 3.]], ['class1', 'class2'])
 
 def test_classification_sample_weight():
     X = [[0], [0], [1]]


Code review comments:
diff_hunk: @@ -143,13 +143,17 @@ def fit(self, X, y, sample_weight=None):
          self.n_classes_,
          self.class_prior_) = class_distribution(y, sample_weight)
 
-        if (self.strategy == "constant" and
-                any(constant[k] not in self.classes_[k]
-                    for k in range(self.n_outputs_))):
-            # Checking in case of constant strategy if the constant
-            # provided by the user is in y.
-            raise ValueError("The constant target value must be "
-                             "present in training data")
+        if self.strategy == "constant":
+            for k in range(self.n_outputs_):
+                if constant[k] not in self.classes_[k]:
+                    # Checking in case of constant strategy if the constant
+                    # provided by the user is in y.
+                    raise ValueError(
+                        "The constant target value must be present "
+                        "in the training data. "
+                        "You provided constant=%r. "
+                        "Possible values are: %r." % (self.constant,
+                                                      list(self.classes_[k])))
path: sklearn/dummy.py
text: @user1:
why ńot using formating? 

```py
_ERR_MSG = (
"The constant ... "
" Possible values (from training data) are: {possible_values}"
" You provided {provided}."
).format(possible_values=list(self.classes_[k]), provided=self.constant)
```

and then just call `raise ValueError(_ERR_MSG)`

@author:
I used the % approach to keep consistency with the rest of the script. But I will define the error message separately to improve readability.  

@user2:
I think this is not quite usual in scikit-learn:

```python
err_msg = ("The constant target value must be present in the training target."
           "Got {} while expecting one of the following: {}
           .format(self.constant, self.classes_[k])".)
```

@user1:
@author if `format` is used nowhere else in `sklearn/dummy.py` I would then use `%` style. If format is already there then I would use format.

It is true that `err_msg` is better than `_MSG`.

diff_hunk: @@ -543,6 +543,11 @@ def test_constant_strategy_exceptions():
                           constant=[2, 0])
     assert_raises(ValueError, clf.fit, X, y)
 
+    clf = DummyClassifier(strategy='constant', constant='not-in-dataset')
+    with pytest.raises(ValueError,
+                       match="constant='not-in-dataset'.+"
+                       "Possible values.+['class1', 'class2']"):
path: sklearn/tests/test_dummy.py
text: @user1:
I find this context manager difficult to read. the `+` seems to add the strings rather than part of the regex. And I would use a more clearly invalid constant name.

maybe:
```py
_MSG = "constant.*must be present in.*train.*data .* provided .* 'foo'.*Possible.*['class1', 'class2']"
with pytest.raises(ValueError, match=_MSG):
    DummyClassifer(
        strategy='constant', constant='foo',
    ).fit(X=np.empty((2,2)), y=['class1', 'class2'])
```

@user2:
Since you are modifying the test, it is a good occasion to parametrize it