chore(skore): Moving confusion matrix display kwargs into set_style (#2293)

Partially address #1957 

This PR add a specialized `set_style` for the `ConfusionMatrixDisplay`
such that we move the kwargs specifically in this function instead of
putting it inside the `plot` one to simplify the user API.

The option `policy` is also set to `"update"` by default because it has
a better behaviour by default by not overriding all the default
parameter already set.

Author: glemaitre

examples/model_evaluation/plot_estimator_report.py

@@ -418,7 +418,7 @@ def operational_decision_cost(y_true, y_pred, amount):
 # %%
 # More plotting options are available via ``heatmap_kwargs``, which are passed to
 # seaborn's heatmap. For example, we can customize the colormap and number format:
-cm_display.plot(heatmap_kwargs={"cmap": "Greens", "fmt": ".2e"})
+cm_display.set_style(heatmap_kwargs={"cmap": "Greens", "fmt": ".2e"}).plot()
 plt.show()
 
 # %%

skore/src/skore/_sklearn/_plot/base.py

@@ -29,7 +29,9 @@ class Display(Protocol):
     def plot(self, **kwargs: Any) -> None:
         """Display a figure containing the information of the display."""
 
-    def set_style(self, **kwargs: Any) -> None:
+    def set_style(
+        self, *, policy: Literal["override", "update"] = "update", **kwargs: Any
+    ) -> None:
         """Set the style of the display."""
 
     def frame(self, **kwargs: Any) -> pd.DataFrame:
@@ -136,13 +138,13 @@ def _style_params(self) -> list[str]:
         ]
 
     def set_style(
-        self, *, policy: Literal["override", "update"] = "override", **kwargs: Any
+        self, *, policy: Literal["override", "update"] = "update", **kwargs: Any
     ):
         """Set the style parameters for the display.
 
         Parameters
         ----------
-        policy : Literal["override", "update"], default="override"
+        policy : Literal["override", "update"], default="update"
             Policy to use when setting the style parameters.
             If "override", existing settings are set to the provided values.
             If "update", existing settings are not changed; only settings that were

skore/src/skore/_sklearn/_plot/metrics/confusion_matrix.py

@@ -106,8 +106,6 @@ def plot(
         normalize: Literal["true", "pred", "all"] | None = None,
         threshold_value: float | None = None,
         subplot_by: Literal["split", "estimator", "auto"] | None = "auto",
-        heatmap_kwargs: dict | None = None,
-        facet_grid_kwargs: dict | None = None,
     ):
         """Plot the confusion matrix.
 
@@ -135,14 +133,6 @@ def plot(
             be subplotted. If "auto", the variable will be automatically determined
             based on the report type.
 
-        heatmap_kwargs : dict, default=None
-            Additional keyword arguments to be passed to seaborn's
-            :func:`seaborn.heatmap`.
-
-        facet_grid_kwargs : dict, default=None
-            Additional keyword arguments to be passed to seaborn's
-            :class:`seaborn.FacetGrid`.
-
         Returns
         -------
         self : ConfusionMatrixDisplay
@@ -152,8 +142,6 @@ def plot(
             normalize=normalize,
             threshold_value=threshold_value,
             subplot_by=subplot_by,
-            heatmap_kwargs=heatmap_kwargs,
-            facet_grid_kwargs=facet_grid_kwargs,
         )
 
     def _plot_matplotlib(
@@ -162,8 +150,6 @@ def _plot_matplotlib(
         normalize: Literal["true", "pred", "all"] | None = None,
         threshold_value: float | None = None,
         subplot_by: Literal["split", "estimator", "auto"] | None = "auto",
-        heatmap_kwargs: dict | None = None,
-        facet_grid_kwargs: dict | None = None,
     ) -> None:
         """Matplotlib implementation of the `plot` method.
 
@@ -184,28 +170,13 @@ def _plot_matplotlib(
             The variable to use for subplotting. If None, the confusion matrix will not
             be subplotted. If "auto", the variable will be automatically determined
             based on the report type.
-
-        heatmap_kwargs : dict, default=None
-            Additional keyword arguments to be passed to seaborn's
-            :func:`seaborn.heatmap`.
-
-        facet_grid_kwargs : dict, default=None
-            Additional keyword arguments to be passed to seaborn's
-            :class:`seaborn.FacetGrid`.
         """
         subplot_by_validated = self._validate_subplot_by(subplot_by, self.report_type)
 
         if "cross-validation" in self.report_type and subplot_by_validated != "split":
             # Aggregate the data across splits and create custom annotations.
             default_fmt = ".3f" if normalize else ".1f"
-            annot_fmt = (
-                heatmap_kwargs.pop("fmt", default_fmt)
-                if heatmap_kwargs
-                else self._default_heatmap_kwargs.pop("fmt", default_fmt)
-                # if fmt was changed with set_style
-                if "fmt" in self._default_heatmap_kwargs
-                else default_fmt
-            )
+            annot_fmt = self._default_heatmap_kwargs.get("fmt", default_fmt)
             frame = self.frame(normalize=normalize, threshold_value=threshold_value)
             aggregated = (
                 frame.groupby(
@@ -226,13 +197,11 @@ def _plot_matplotlib(
             default_fmt = ".2f" if normalize else "d"
 
         heatmap_kwargs_validated = _validate_style_kwargs(
-            {"fmt": default_fmt, **self._default_heatmap_kwargs},
-            heatmap_kwargs or {},
+            {"fmt": default_fmt, **self._default_heatmap_kwargs}, {}
         )
 
         facet_grid_kwargs_validated = _validate_style_kwargs(
-            {"col": subplot_by_validated, **self._default_facet_grid_kwargs},
-            facet_grid_kwargs or {},
+            {"col": subplot_by_validated, **self._default_facet_grid_kwargs}, {}
         )
         grid = sns.FacetGrid(
             data=frame,
@@ -605,3 +574,46 @@ def select_threshold_and_format(group):
             frames.append(select_threshold_and_format(self.confusion_matrix))
 
         return pd.concat(frames)
+
+    # ignore the type signature because we override kwargs by specifying the name of
+    # the parameters for the user.
+    def set_style(  # type: ignore[override]
+        self,
+        *,
+        policy: Literal["override", "update"] = "update",
+        heatmap_kwargs: dict | None = None,
+        facet_grid_kwargs: dict | None = None,
+    ):
+        """Set the style parameters for the display.
+
+        Parameters
+        ----------
+        policy : Literal["override", "update"], default="update"
+            Policy to use when setting the style parameters.
+            If "override", existing settings are set to the provided values.
+            If "update", existing settings are not changed; only settings that were
+            previously unset are changed.
+
+        heatmap_kwargs : dict, default=None
+            Additional keyword arguments to be passed to seaborn's
+            :func:`seaborn.heatmap`.
+
+        facet_grid_kwargs : dict, default=None
+            Additional keyword arguments to be passed to seaborn's
+            :class:`seaborn.FacetGrid`.
+
+        Returns
+        -------
+        self : object
+            Returns the instance itself.
+
+        Raises
+        ------
+        ValueError
+            If a style parameter is unknown.
+        """
+        return super().set_style(
+            policy=policy,
+            heatmap_kwargs=heatmap_kwargs or {},
+            facet_grid_kwargs=facet_grid_kwargs or {},
+        )

skore/tests/unit/displays/confusion_matrix/test_common.py

@@ -109,7 +109,7 @@ def test_facet_grid_kwargs(pyplot, fixture_prefix, task, request):
         display.plot()
         assert display.figure_.get_figheight() == 6
 
-        display.plot(facet_grid_kwargs={"height": 8})
+        display.set_style(facet_grid_kwargs={"height": 8}).plot()
         assert display.figure_.get_figheight() == 8
 
     @pytest.mark.parametrize("task", ["binary", "multiclass"])
@@ -130,15 +130,13 @@ def get_ax(display):
         display = report.metrics.confusion_matrix()
         display.plot()
         assert get_ax(display).collections[0].get_cmap().name == "Blues"
-        display.plot(heatmap_kwargs={"cmap": "Reds"})
+        display.set_style(heatmap_kwargs={"cmap": "Reds"}).plot()
         assert get_ax(display).collections[0].get_cmap().name == "Reds"
-        display.set_style(heatmap_kwargs={"cmap": "Greens"}, policy="update").plot()
-        assert get_ax(display).collections[0].get_cmap().name == "Greens"
 
         display = report.metrics.confusion_matrix()
         display.plot()
         assert len(get_ax(display).texts) > 1
-        display.plot(heatmap_kwargs={"annot": False})
+        display.set_style(heatmap_kwargs={"annot": False}).plot()
         # There is still the pos_label annotation
         assert len(get_ax(display).texts) == n_base_elements
         plt.close("all")
@@ -148,25 +146,17 @@ def get_ax(display):
         for text in get_ax(display).texts:
             text_content = text.get_text()
             assert "." in text_content or "*" in text_content
-        display.plot(normalize="all", heatmap_kwargs={"fmt": ".2e"})
+        display.set_style(heatmap_kwargs={"fmt": ".2e"}).plot(normalize="all")
         for text in get_ax(display).texts:
             text_content = text.get_text()
             assert "e" in text_content
-        display.set_style(heatmap_kwargs={"fmt": ".2E"}, policy="update").plot(
-            normalize="all"
-        )
-        for text in get_ax(display).texts:
-            text_content = text.get_text()
-            assert "E" in text_content or "*" in text_content
         plt.close("all")
 
         display = report.metrics.confusion_matrix()
         display.plot()
         assert len(display.figure_.axes) == n_plots
-        display.plot(heatmap_kwargs={"cbar": True})
+        display.set_style(heatmap_kwargs={"cbar": True}).plot()
         assert len(display.figure_.axes) == 2 * n_plots
-        display.set_style(heatmap_kwargs={"cbar": False}, policy="update").plot()
-        assert len(display.figure_.axes) == n_plots
         plt.close("all")
 
     @pytest.mark.parametrize("task", ["binary", "multiclass"])

skore/tests/unit/displays/test_style.py

@@ -21,7 +21,7 @@ def test_style_mixin():
     "initial_state, override_value, expected_result, use_explicit_policy",
     [
         (None, {"a": 1, "b": 2}, {"a": 1, "b": 2}, False),
-        ({"a": 1, "b": 2}, {"c": 3}, {"c": 3}, False),
+        ({"a": 1, "b": 2}, {"c": 3}, {"a": 1, "b": 2, "c": 3}, False),
         ({"c": 3}, {"a": 1, "b": 2}, {"a": 1, "b": 2}, True),
         ({"a": 1, "b": 2}, 42, 42, True),
     ],