Repository: scikit-learn/scikit-learn
Pull request: FIX Uses the color max for colormap in ConfusionMatrixDisplay
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/main/CONTRIBUTING.md
-->

#### Reference Issues/PRs
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->
Fixes https://github.com/scikit-learn/scikit-learn/issues/19777


#### What does this implement/fix? Explain your changes.
Uses the color max for colormap in `ConfusionMatrixDisplay`.

<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
ConfusionMatrixDisplay miscolors text if colormap has more than 256 colors
#### Describe the bug
When ConfusionMatrixDisplay is deciding on text colors, it uses colormap entry 256 as the text color for labeling cells below the mean value. This is line 125 of metrics/_plot/confusion_matrix.py:
`cmap_min, cmap_max = self.im_.cmap(0), self.im_.cmap(256)`

This causes incorrect coloring and hard-to-read graphs if using a ListedColormap with more than 256 entries. (Which was necessary for my application because I needed to renormalize the colormap)

#### Steps/Code to Reproduce
```python
import numpy as np
import matplotlib
import matplotlib.cm
from sklearn.metrics import ConfusionMatrixDisplay

def getMapWith(colors):
    ans = matplotlib.cm.get_cmap('gray')(np.linspace(0, 1, colors)) ** 0.5 
    return matplotlib.colors.ListedColormap(ans)
x = np.random.rand(3, 3)
disp = ConfusionMatrixDisplay(x)

disp.plot(cmap=getMapWith(256)) # Works great
disp.plot(cmap=getMapWith(1024)) # Can't read
```
#### Versions
```
System:
    python: 3.8.5 (default, Jul 28 2020, 12:59:40)  [GCC 9.3.0]
executable: /home/mcrouch/test2/env/bin/python
   machine: Linux-5.4.0-47-generic-x86_64-with-glibc2.29

Python dependencies:
          pip: 21.0.1
   setuptools: 54.1.2
      sklearn: 0.24.1
        numpy: 1.19.5
        scipy: 1.6.1
       Cython: None
       pandas: 1.2.3
   matplotlib: 3.3.4
       joblib: 1.0.1
threadpoolctl: 2.1.0

Built with OpenMP: True
```

Hints from issue discussion:

Commit message: Merge remote-tracking branch 'upstream/main' into confusion_matrix_max

Patch to review:
diff --git a/doc/whats_new/v1.0.rst b/doc/whats_new/v1.0.rst
index d26c5dd0c347d..08f6d34c6a379 100644
--- a/doc/whats_new/v1.0.rst
+++ b/doc/whats_new/v1.0.rst
@@ -324,6 +324,9 @@ Changelog
   are integral.
   :pr:`9843` by :user:`Jon Crall <Erotemic>`.
 
+- |Fix| :meth:`metrics.ConfusionMatrixDisplay.plot` uses the correct max
+  for colormap. :pr:`19784` by `Thomas Fan`_.
+
 - |Fix| Samples with zero `sample_weight` values do not affect the results
   from :func:`metrics.det_curve`, :func:`metrics.precision_recall_curve`
   and :func:`metrics.roc_curve`.
diff --git a/sklearn/metrics/_plot/confusion_matrix.py b/sklearn/metrics/_plot/confusion_matrix.py
index 9fcecec775e6e..891ceede25b9d 100644
--- a/sklearn/metrics/_plot/confusion_matrix.py
+++ b/sklearn/metrics/_plot/confusion_matrix.py
@@ -122,7 +122,7 @@ def plot(self, *, include_values=True, cmap='viridis',
         n_classes = cm.shape[0]
         self.im_ = ax.imshow(cm, interpolation='nearest', cmap=cmap)
         self.text_ = None
-        cmap_min, cmap_max = self.im_.cmap(0), self.im_.cmap(256)
+        cmap_min, cmap_max = self.im_.cmap(0), self.im_.cmap(1.0)
 
         if include_values:
             self.text_ = np.empty_like(cm, dtype=object)
diff --git a/sklearn/metrics/_plot/tests/test_confusion_matrix_display.py b/sklearn/metrics/_plot/tests/test_confusion_matrix_display.py
index ed0bc04117396..ed5d89858fb20 100644
--- a/sklearn/metrics/_plot/tests/test_confusion_matrix_display.py
+++ b/sklearn/metrics/_plot/tests/test_confusion_matrix_display.py
@@ -380,3 +380,17 @@ def test_confusion_matrix_with_unknown_labels(pyplot, constructor_name):
     display_labels = [tick.get_text() for tick in disp.ax_.get_xticklabels()]
     expected_labels = [str(i) for i in range(n_classes + 1)]
     assert_array_equal(expected_labels, display_labels)
+
+
+def test_colormap_max(pyplot):
+    """Check that the max color is used for the color of the text."""
+
+    from matplotlib import cm
+    gray = cm.get_cmap('gray', 1024)
+    cm = np.array([[1.0, 0.0], [0.0, 1.0]])
+
+    disp = ConfusionMatrixDisplay(cm)
+    disp.plot(cmap=gray)
+
+    color = disp.text_[1, 0].get_color()
+    assert_allclose(color, [1.0, 1.0, 1.0, 1.0])


Code review comments:
diff_hunk: @@ -380,3 +380,17 @@ def test_confusion_matrix_with_unknown_labels(pyplot, constructor_name):
     display_labels = [tick.get_text() for tick in disp.ax_.get_xticklabels()]
     expected_labels = [str(i) for i in range(n_classes + 1)]
     assert_array_equal(expected_labels, display_labels)
+
+
+def test_colormap_max(pyplot):
+    """Check that the max color is used for the color of the text."""
+
+    from matplotlib import cm
+    gray = cm.get_cmap('gray', 1024)
+    cm = np.array([[1.0, 0.0], [0.0, 1.0]])
+
+    disp = ConfusionMatrixDisplay(cm)
path: sklearn/metrics/_plot/tests/test_confusion_matrix_display.py
text: @user1:
```suggestion
    confusion_matrix = np.array([[1.0, 0.0], [0.0, 1.0]])

    disp = ConfusionMatrixDisplay(confusion_matrix)
```