Repository: scikit-learn/scikit-learn
Pull request: MNT Raise erorr when normalize is invalid in confusion_matrix
Fixes #15886

Problem statement:
confusion_matrix doesn't validate normalize keyword
```python
import numpy as np
from sklearn.metrics import confusion_matrix
confusion_matrix(np.random.randint(0, 5, size=100),
                 np.random.randint(0, 5, size=100), normalize='ohnodontdoit')
```

should raise an error but runs fine with no normalization. I found this because I did ``normalize=True`` and it didn't do anything and didn't raise an error.

Hints from issue discussion:
True we forgot to move the checking which is actually happening in `plot_confusion_matrix` (l. 187). This should be moved in `confusion_matrix`.

Commit message: remove

Patch to review:
diff --git a/sklearn/metrics/_classification.py b/sklearn/metrics/_classification.py
index 322ac3409722f..6e6bc839dc95d 100644
--- a/sklearn/metrics/_classification.py
+++ b/sklearn/metrics/_classification.py
@@ -283,6 +283,10 @@ def confusion_matrix(y_true, y_pred, labels=None, sample_weight=None,
 
     check_consistent_length(y_true, y_pred, sample_weight)
 
+    if normalize not in {'true', 'pred', 'all', None}:
+        raise ValueError("normalize must be one of {'true', 'pred', "
+                         "'all', None}")
+
     n_labels = labels.size
     label_to_ind = {y: x for x, y in enumerate(labels)}
     # convert yt, yp into index
diff --git a/sklearn/metrics/_plot/confusion_matrix.py b/sklearn/metrics/_plot/confusion_matrix.py
index be59c8dd9a847..9eec258ef69ce 100644
--- a/sklearn/metrics/_plot/confusion_matrix.py
+++ b/sklearn/metrics/_plot/confusion_matrix.py
@@ -184,10 +184,6 @@ def plot_confusion_matrix(estimator, X, y_true, labels=None,
     if not is_classifier(estimator):
         raise ValueError("plot_confusion_matrix only supports classifiers")
 
-    if normalize not in {'true', 'pred', 'all', None}:
-        raise ValueError("normalize must be one of {'true', 'pred', "
-                         "'all', None}")
-
     y_pred = estimator.predict(X)
     cm = confusion_matrix(y_true, y_pred, sample_weight=sample_weight,
                           labels=labels, normalize=normalize)
diff --git a/sklearn/metrics/tests/test_classification.py b/sklearn/metrics/tests/test_classification.py
index 4c1db4b55bb16..c33c3a829cc16 100644
--- a/sklearn/metrics/tests/test_classification.py
+++ b/sklearn/metrics/tests/test_classification.py
@@ -526,6 +526,13 @@ def test_confusion_matrix_normalize(normalize, cm_dtype, expected_results):
     assert cm.dtype.kind == cm_dtype
 
 
+def test_confusion_matrix_normalize_wrong_option():
+    y_test = [0, 0, 0, 0, 1, 1, 1, 1]
+    y_pred = [0, 0, 0, 0, 0, 0, 0, 0]
+    with pytest.raises(ValueError, match='normalize must be one of'):
+        confusion_matrix(y_test, y_pred, normalize=True)
+
+
 def test_confusion_matrix_normalize_single_class():
     y_test = [0, 0, 0, 0, 1, 1, 1, 1]
     y_pred = [0, 0, 0, 0, 0, 0, 0, 0]


Code review comments:
diff_hunk: @@ -283,6 +283,10 @@ def confusion_matrix(y_true, y_pred, labels=None, sample_weight=None,
 
     check_consistent_length(y_true, y_pred, sample_weight)
 
+    if normalize not in {'true', 'pred', 'all', None}:
path: sklearn/metrics/_classification.py
text: @user1:
```suggestion
    if normalize not in ['true', 'pred', 'all', None]:
```
as otherwise you would get a type error if `normalize` is unhashable,
```
>>> {'a': 2} in {'true', 'false'}
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'dict'
>>> {'a': 2} in ['true', 'false']
False
```