Refactor of colorbar and norm logic (#346)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Wouter-Michiel Vierdag <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -18,11 +18,15 @@ and this project adheres to [Semantic Versioning][].
 
 -   Lowered RMSE-threshold for plot-based tests from 45 to 15 (#344)
 -   When subsetting to `groups`, `NA` isn't automatically added to legend (#344)
+-   When rendering a single image channel, a colorbar is now shown (#346)
+-   Removed `percentiles_for_norm` parameter (#346)
+-   Changed `norm` to no longer accept bools, only `mpl.colors.Normalise` or `None` (#346)
 
 ### Fixed
 
 -   Filtering with `groups` now preserves original cmap (#344)
 -   Non-selected `groups` are now not shown in `na_color` (#344)
+-   Several issues associated with `norm` and `colorbar` (#346)
 
 ## [0.2.5] - 2024-08-23
 

src/spatialdata_plot/pl/basic.py

@@ -166,7 +166,7 @@ def render_shapes(
         outline_color: str | list[float] = "#000000ff",
         outline_alpha: float | int = 0.0,
         cmap: Colormap | str | None = None,
-        norm: bool | Normalize = False,
+        norm: Normalize | None = None,
         scale: float | int = 1.0,
         method: str | None = None,
         table_name: str | None = None,
@@ -301,7 +301,7 @@ def render_points(
         palette: list[str] | str | None = None,
         na_color: ColorLike | None = "default",
         cmap: Colormap | str | None = None,
-        norm: None | Normalize = None,
+        norm: Normalize | None = None,
         size: float | int = 1.0,
         method: str | None = None,
         table_name: str | None = None,
@@ -422,7 +422,6 @@ def render_images(
         na_color: ColorLike | None = "default",
         palette: list[str] | str | None = None,
         alpha: float | int = 1.0,
-        percentiles_for_norm: tuple[float, float] | None = None,
         scale: str | None = None,
         **kwargs: Any,
     ) -> sd.SpatialData:
@@ -457,8 +456,6 @@ def render_images(
             Palette to color images. The number of palettes should be equal to the number of channels.
         alpha : float | int, default 1.0
             Alpha value for the images. Must be a numeric between 0 and 1.
-        percentiles_for_norm : tuple[float, float] | None
-            Optional pair of floats (pmin < pmax, 0-100) which will be used for quantile normalization.
         scale : str | None
             Influences the resolution of the rendering. Possibilities include:
                 1) `None` (default): The image is rasterized to fit the canvas size. For
@@ -486,20 +483,14 @@ def render_images(
             cmap=cmap,
             norm=norm,
             scale=scale,
-            percentiles_for_norm=percentiles_for_norm,
         )
 
         sdata = self._copy()
         sdata = _verify_plotting_tree(sdata)
         n_steps = len(sdata.plotting_tree.keys())
 
         for element, param_values in params_dict.items():
-            # cmap_params = _prepare_cmap_norm(
-            #     cmap=params_dict[element]["cmap"],
-            #     norm=norm,
-            #     na_color=params_dict[element]["na_color"],  # type: ignore[arg-type]
-            #     **kwargs,
-            # )
+
             cmap_params: list[CmapParams] | CmapParams
             if isinstance(cmap, list):
                 cmap_params = [
@@ -525,7 +516,6 @@ def render_images(
                 cmap_params=cmap_params,
                 palette=param_values["palette"],
                 alpha=param_values["alpha"],
-                percentiles_for_norm=param_values["percentiles_for_norm"],
                 scale=param_values["scale"],
                 zorder=n_steps,
             )

src/spatialdata_plot/pl/render.py

@@ -9,6 +9,7 @@
 import datashader as ds
 import geopandas as gpd
 import matplotlib
+import matplotlib.pyplot as plt
 import matplotlib.transforms as mtransforms
 import numpy as np
 import pandas as pd
@@ -47,7 +48,6 @@
     _maybe_set_colors,
     _mpl_ax_contains_elements,
     _multiscale_to_spatial_image,
-    _normalize,
     _rasterize_if_necessary,
     _set_color_source_vec,
     to_hex,
@@ -128,6 +128,7 @@ def _render_shapes(
         shapes = shapes.reset_index()
         color_source_vector = color_source_vector[mask]
         color_vector = color_vector[mask]
+
     shapes = gpd.GeoDataFrame(shapes, geometry="geometry")
 
     # Using dict.fromkeys here since set returns in arbitrary order
@@ -255,9 +256,13 @@ def _render_shapes(
         for path in _cax.get_paths():
             path.vertices = trans.transform(path.vertices)
 
-    # Sets the limits of the colorbar to the values instead of [0, 1]
-    if not norm and not values_are_categorical:
-        _cax.set_clim(min(color_vector), max(color_vector))
+    if not values_are_categorical:
+        # If the user passed a Normalize object with vmin/vmax we'll use those,
+        # # if not we'll use the min/max of the color_vector
+        _cax.set_clim(
+            vmin=render_params.cmap_params.norm.vmin or min(color_vector),
+            vmax=render_params.cmap_params.norm.vmax or max(color_vector),
+        )
 
     if len(set(color_vector)) != 1 or list(set(color_vector))[0] != to_hex(render_params.cmap_params.na_color):
         # necessary in case different shapes elements are annotated with one table
@@ -603,11 +608,6 @@ def _render_images(
     if n_channels == 1 and not isinstance(render_params.cmap_params, list):
         layer = img.sel(c=channels[0]).squeeze() if isinstance(channels[0], str) else img.isel(c=channels[0]).squeeze()
 
-        if render_params.percentiles_for_norm != (None, None):
-            layer = _normalize(
-                layer, pmin=render_params.percentiles_for_norm[0], pmax=render_params.percentiles_for_norm[1], clip=True
-            )
-
         if render_params.cmap_params.norm:  # type: ignore[attr-defined]
             layer = render_params.cmap_params.norm(layer)  # type: ignore[attr-defined]
 
@@ -623,20 +623,16 @@ def _render_images(
 
         _ax_show_and_transform(layer, trans_data, ax, cmap=cmap, zorder=render_params.zorder)
 
+        if legend_params.colorbar:
+            sm = plt.cm.ScalarMappable(cmap=cmap, norm=render_params.cmap_params.norm)
+            fig_params.fig.colorbar(sm, ax=ax)
+
     # 2) Image has any number of channels but 1
     else:
         layers = {}
         for ch_index, c in enumerate(channels):
             layers[c] = img.sel(c=c).copy(deep=True).squeeze()
 
-            if render_params.percentiles_for_norm != (None, None):
-                layers[c] = _normalize(
-                    layers[c],
-                    pmin=render_params.percentiles_for_norm[0],
-                    pmax=render_params.percentiles_for_norm[1],
-                    clip=True,
-                )
-
             if not isinstance(render_params.cmap_params, list):
                 if render_params.cmap_params.norm is not None:
                     layers[c] = render_params.cmap_params.norm(layers[c])

src/spatialdata_plot/pl/utils.py

@@ -489,7 +489,7 @@ def _get_scalebar(
 
 def _prepare_cmap_norm(
     cmap: Colormap | str | None = None,
-    norm: Normalize | bool = False,
+    norm: Normalize | None = None,
     na_color: ColorLike | None = None,
     vmin: float | None = None,
     vmax: float | None = None,
@@ -1623,29 +1623,6 @@ def _type_check_params(param_dict: dict[str, Any], element_type: str) -> dict[st
             if scale < 0:
                 raise ValueError("Parameter 'scale' must be a positive number.")
 
-    if (percentiles_for_norm := param_dict.get("percentiles_for_norm")) is None:
-        percentiles_for_norm = (None, None)
-    elif not (isinstance(percentiles_for_norm, (list, tuple)) or len(percentiles_for_norm) != 2):
-        raise TypeError("Parameter 'percentiles_for_norm' must be a list or tuple of exactly two floats or None.")
-    elif not all(
-        isinstance(p, (float, int, type(None)))
-        and isinstance(p, type(percentiles_for_norm[0]))
-        and (p is None or 0 <= p <= 100)
-        for p in percentiles_for_norm
-    ):
-        raise TypeError(
-            "Each item in 'percentiles_for_norm' must be of the same dtype and must be a float or int within [0, 100], "
-            "or None"
-        )
-    elif (
-        percentiles_for_norm[0] is not None
-        and percentiles_for_norm[1] is not None
-        and percentiles_for_norm[0] > percentiles_for_norm[1]
-    ):
-        raise ValueError("The first number in 'percentiles_for_norm' must not be smaller than the second.")
-    if "percentiles_for_norm" in param_dict:
-        param_dict["percentiles_for_norm"] = percentiles_for_norm
-
     if size := param_dict.get("size"):
         if not isinstance(size, (float, int)):
             raise TypeError("Parameter 'size' must be numeric.")
@@ -1886,7 +1863,6 @@ def _validate_image_render_params(
     cmap: list[Colormap | str] | Colormap | str | None,
     norm: Normalize | None,
     scale: str | None,
-    percentiles_for_norm: tuple[float | None, float | None] | None,
 ) -> dict[str, dict[str, Any]]:
     param_dict: dict[str, Any] = {
         "sdata": sdata,
@@ -1898,7 +1874,6 @@ def _validate_image_render_params(
         "cmap": cmap,
         "norm": norm,
         "scale": scale,
-        "percentiles_for_norm": percentiles_for_norm,
     }
     param_dict = _type_check_params(param_dict, "images")
 
@@ -1945,8 +1920,6 @@ def _validate_image_render_params(
         else:
             element_params[el]["scale"] = scale
 
-        element_params[el]["percentiles_for_norm"] = param_dict["percentiles_for_norm"]
-
     return element_params