incorporate review feedback

Author: Sonja Stockhaus

src/spatialdata_plot/pl/render.py

@@ -37,7 +37,7 @@
     _ax_show_and_transform,
     _create_image_from_datashader_result,
     _datashader_aggregate_with_function,
-    _datashader_shade,
+    _datashader_map_aggregate_to_color,
     _datshader_get_how_kw_for_spread,
     _decorate_axs,
     _get_collection_shape,
@@ -259,11 +259,12 @@ def _render_shapes(
                 if isinstance(ds_cmap, str) and ds_cmap[0] == "#":
                     ds_cmap = ds_cmap[:-2]
 
-            ds_result = _datashader_shade(
+            ds_result = _datashader_map_aggregate_to_color(
                 agg,
                 cmap=ds_cmap,
                 color_key=color_key,
-                min_alpha=np.min([254, render_params.fill_alpha * 255]),
+                # min_alpha=np.min([254, render_params.fill_alpha * 255]),
+                min_alpha=render_params.fill_alpha * 255,
             )
         elif aggregate_with_reduction is not None:  # to shut up mypy
             ds_cmap = render_params.cmap_params.cmap
@@ -274,10 +275,11 @@ def _render_shapes(
                 ds_cmap = matplotlib.colors.to_hex(render_params.cmap_params.cmap(0.0), keep_alpha=False)
                 aggregate_with_reduction = (aggregate_with_reduction[0], aggregate_with_reduction[0] + 1)
 
-            ds_result = _datashader_shade(
+            ds_result = _datashader_map_aggregate_to_color(
                 agg,
                 cmap=ds_cmap,
-                min_alpha=np.min([254, render_params.fill_alpha * 255]),
+                # min_alpha=np.min([254, render_params.fill_alpha * 255]),
+                min_alpha=render_params.fill_alpha * 255,
                 span=ds_span,
                 clip=norm.clip,
             )
@@ -295,7 +297,8 @@ def _render_shapes(
             ds_outlines = ds.tf.shade(
                 agg_outlines,
                 cmap=outline_color,
-                min_alpha=np.min([254, render_params.outline_alpha * 255]),
+                # min_alpha=np.min([254, render_params.outline_alpha * 255]),
+                min_alpha=render_params.outline_alpha * 255,
                 how="linear",
             )
 
@@ -621,11 +624,12 @@ def _render_points(
             color_vector = np.asarray([x[:-2] for x in color_vector])
 
         if color_by_categorical or col_for_color is None:
-            ds_result = _datashader_shade(
+            ds_result = _datashader_map_aggregate_to_color(
                 ds.tf.spread(agg, px=px),
                 cmap=color_vector[0],
                 color_key=color_key,
-                min_alpha=np.min([254, render_params.alpha * 255]),
+                # min_alpha=np.min([254, render_params.alpha * 255]),
+                min_alpha=render_params.alpha * 255,
             )
         else:
             spread_how = _datshader_get_how_kw_for_spread(render_params.ds_reduction)
@@ -640,12 +644,13 @@ def _render_points(
                 ds_cmap = matplotlib.colors.to_hex(render_params.cmap_params.cmap(0.0), keep_alpha=False)
                 aggregate_with_reduction = (aggregate_with_reduction[0], aggregate_with_reduction[0] + 1)
 
-            ds_result = _datashader_shade(
+            ds_result = _datashader_map_aggregate_to_color(
                 agg,
                 cmap=ds_cmap,
                 span=ds_span,
                 clip=norm.clip,
-                min_alpha=np.min([254, render_params.alpha * 255]),
+                # min_alpha=np.min([254, render_params.alpha * 255]),
+                min_alpha=render_params.alpha * 255,
             )
 
         rgba_image, trans_data = _create_image_from_datashader_result(ds_result, factor, ax)
@@ -665,12 +670,6 @@ def _render_points(
             if (norm.vmin is not None or norm.vmax is not None) and norm.vmin == norm.vmax:
                 vmin = norm.vmin - 0.5
                 vmax = norm.vmin + 0.5
-                # if norm.clip:
-                #     vmin = norm.vmin
-                #     vmax = norm.vmin + 1
-                # else:
-                #     vmin = norm.vmin - 0.5
-                #     vmax = norm.vmin + 0.5
             cax = ScalarMappable(
                 norm=matplotlib.colors.Normalize(vmin=vmin, vmax=vmax),
                 cmap=render_params.cmap_params.cmap,
@@ -804,6 +803,7 @@ def _render_images(
         cmap._init()
         cmap._lut[:, -1] = render_params.alpha
 
+        # norm needs to be passed directly to ax.imshow(). If we normalize before, that method would always clip.
         _ax_show_and_transform(
             layer, trans_data, ax, cmap=cmap, zorder=render_params.zorder, norm=render_params.cmap_params.norm
         )

src/spatialdata_plot/pl/utils.py

@@ -2271,7 +2271,7 @@ def _get_transformation_matrix_for_datashader(
     return _get_datashader_trans_matrix_of_single_element(trans)
 
 
-def _datashader_shade(
+def _datashader_map_aggregate_to_color(
     agg: DataArray,
     cmap: str | list[str] | ListedColormap,
     color_key: None | list[str] = None,

tests/conftest.py

@@ -3,6 +3,7 @@
 from functools import wraps
 from pathlib import Path
 
+import matplotlib
 import matplotlib.pyplot as plt
 import numpy as np
 import pandas as pd
@@ -149,6 +150,23 @@ def test_sdata_multiple_images_diverging_dims():
     return sdata
 
 
+@pytest.fixture
+def sdata_blobs_shapes_annotated() -> SpatialData:
+    """Get blobs sdata with continuous annotation of polygons."""
+    blob = blobs()
+    blob["table"].obs["region"] = "blobs_polygons"
+    blob["table"].uns["spatialdata_attrs"]["region"] = "blobs_polygons"
+    blob.shapes["blobs_polygons"]["value"] = [1, 2, 3, 4, 5]
+    return blob
+
+
+def _viridis_with_under_over() -> matplotlib.colors.ListedColormap:
+    cmap = matplotlib.colormaps["viridis"]
+    cmap.set_under("black")
+    cmap.set_over("grey")
+    return cmap
+
+
 # Code below taken from spatialdata main repo
 
 

tests/pl/test_render_images.py

@@ -7,7 +7,7 @@
 from spatialdata import SpatialData
 
 import spatialdata_plot  # noqa: F401
-from tests.conftest import DPI, PlotTester, PlotTesterMeta
+from tests.conftest import DPI, PlotTester, PlotTesterMeta, _viridis_with_under_over
 
 RNG = np.random.default_rng(seed=42)
 sc.pl.set_rcParams_defaults()
@@ -68,17 +68,15 @@ def test_plot_can_render_two_channels_str_from_multiscale_image(self, sdata_blob
 
     def test_plot_can_pass_normalize_clip_True(self, sdata_blobs: SpatialData):
         norm = Normalize(vmin=0.1, vmax=0.5, clip=True)
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        sdata_blobs.pl.render_images(element="blobs_image", channel=0, norm=norm, cmap=cmap).pl.show()
+        sdata_blobs.pl.render_images(
+            element="blobs_image", channel=0, norm=norm, cmap=_viridis_with_under_over()
+        ).pl.show()
 
     def test_plot_can_pass_normalize_clip_False(self, sdata_blobs: SpatialData):
         norm = Normalize(vmin=0.1, vmax=0.5, clip=False)
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        sdata_blobs.pl.render_images(element="blobs_image", channel=0, norm=norm, cmap=cmap).pl.show()
+        sdata_blobs.pl.render_images(
+            element="blobs_image", channel=0, norm=norm, cmap=_viridis_with_under_over()
+        ).pl.show()
 
     def test_plot_can_pass_color_to_single_channel(self, sdata_blobs: SpatialData):
         sdata_blobs.pl.render_images(element="blobs_image", channel=1, palette="red").pl.show()

tests/pl/test_render_labels.py

@@ -12,7 +12,7 @@
 from spatialdata.models import TableModel
 
 import spatialdata_plot  # noqa: F401
-from tests.conftest import DPI, PlotTester, PlotTesterMeta
+from tests.conftest import DPI, PlotTester, PlotTesterMeta, _viridis_with_under_over
 
 RNG = np.random.default_rng(seed=42)
 sc.pl.set_rcParams_defaults()
@@ -236,19 +236,16 @@ def _make_tablemodel_with_categorical_labels(self, sdata_blobs, labels_name: str
         sdata_blobs["other_table"].obs["category"] = sdata_blobs["other_table"].obs["category"].astype("category")
 
     def test_plot_can_color_with_norm_and_clipping(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_labels(
-            "blobs_labels", color="channel_0_sum", norm=Normalize(400, 1000, clip=True), cmap=cmap
+            "blobs_labels", color="channel_0_sum", norm=Normalize(400, 1000, clip=True), cmap=_viridis_with_under_over()
         ).pl.show()
 
     def test_plot_can_color_with_norm_no_clipping(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_labels(
-            "blobs_labels", color="channel_0_sum", norm=Normalize(400, 1000, clip=False), cmap=cmap
+            "blobs_labels",
+            color="channel_0_sum",
+            norm=Normalize(400, 1000, clip=False),
+            cmap=_viridis_with_under_over(),
         ).pl.show()
 
     def test_plot_can_annotate_labels_with_table_layer(self, sdata_blobs: SpatialData):

tests/pl/test_render_points.py

@@ -14,7 +14,7 @@
 from spatialdata.transformations._utils import _set_transformations
 
 import spatialdata_plot  # noqa: F401
-from tests.conftest import DPI, PlotTester, PlotTesterMeta
+from tests.conftest import DPI, PlotTester, PlotTesterMeta, _viridis_with_under_over
 
 RNG = np.random.default_rng(seed=42)
 sc.pl.set_rcParams_defaults()
@@ -227,67 +227,51 @@ def test_plot_datashader_can_transform_points(self, sdata_blobs: SpatialData):
         sdata_blobs.pl.render_points("blobs_points", method="datashader", color="black", size=5).pl.show()
 
     def test_plot_can_use_norm_with_clip(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        sdata_blobs.pl.render_points(color="instance_id", size=40, norm=Normalize(3, 7, clip=True), cmap=cmap).pl.show()
+        sdata_blobs.pl.render_points(
+            color="instance_id", size=40, norm=Normalize(3, 7, clip=True), cmap=_viridis_with_under_over()
+        ).pl.show()
 
     def test_plot_can_use_norm_without_clip(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_points(
-            color="instance_id", size=40, norm=Normalize(3, 7, clip=False), cmap=cmap
+            color="instance_id", size=40, norm=Normalize(3, 7, clip=False), cmap=_viridis_with_under_over()
         ).pl.show()
 
     def test_plot_datashader_can_use_norm_with_clip(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_points(
             color="instance_id",
             size=40,
             norm=Normalize(3, 7, clip=True),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()
 
     def test_plot_datashader_can_use_norm_without_clip(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_points(
             color="instance_id",
             size=40,
             norm=Normalize(3, 7, clip=False),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()
 
     def test_plot_datashader_norm_vmin_eq_vmax_with_clip(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_points(
             color="instance_id",
             size=40,
             norm=Normalize(5, 5, clip=True),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()
 
     def test_plot_datashader_norm_vmin_eq_vmax_without_clip(self, sdata_blobs: SpatialData):
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
         sdata_blobs.pl.render_points(
             color="instance_id",
             size=40,
             norm=Normalize(5, 5, clip=False),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()

tests/pl/test_render_shapes.py

@@ -16,7 +16,7 @@
 from spatialdata.transformations._utils import _set_transformations
 
 import spatialdata_plot  # noqa: F401
-from tests.conftest import DPI, PlotTester, PlotTesterMeta
+from tests.conftest import DPI, PlotTester, PlotTesterMeta, _viridis_with_under_over
 
 RNG = np.random.default_rng(seed=42)
 sc.pl.set_rcParams_defaults()
@@ -456,82 +456,47 @@ def test_plot_can_do_non_matching_table(self, sdata_blobs: SpatialData):
 
         sdata_blobs.pl.render_shapes("blobs_circles", color="instance_id").pl.show()
 
-    def test_plot_can_color_with_norm_no_clipping(self, sdata_blobs: SpatialData):
-        blob = deepcopy(sdata_blobs)
-        blob["table"].obs["region"] = "blobs_polygons"
-        blob["table"].uns["spatialdata_attrs"]["region"] = "blobs_polygons"
-        blob.shapes["blobs_polygons"]["value"] = [1, 2, 3, 4, 5]
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        blob.pl.render_shapes(
-            element="blobs_polygons", color="value", norm=Normalize(2, 4, clip=False), cmap=cmap
+    def test_plot_can_color_with_norm_no_clipping(self, sdata_blobs_shapes_annotated: SpatialData):
+        sdata_blobs_shapes_annotated.pl.render_shapes(
+            element="blobs_polygons", color="value", norm=Normalize(2, 4, clip=False), cmap=_viridis_with_under_over()
         ).pl.show()
 
-    def test_plot_datashader_can_color_with_norm_and_clipping(self, sdata_blobs: SpatialData):
-        blob = deepcopy(sdata_blobs)
-        blob["table"].obs["region"] = "blobs_polygons"
-        blob["table"].uns["spatialdata_attrs"]["region"] = "blobs_polygons"
-        blob.shapes["blobs_polygons"]["value"] = [1, 2, 3, 4, 5]
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        blob.pl.render_shapes(
+    def test_plot_datashader_can_color_with_norm_and_clipping(self, sdata_blobs_shapes_annotated: SpatialData):
+        sdata_blobs_shapes_annotated.pl.render_shapes(
             element="blobs_polygons",
             color="value",
             norm=Normalize(2, 4, clip=True),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()
 
-    def test_plot_datashader_can_color_with_norm_no_clipping(self, sdata_blobs: SpatialData):
-        blob = deepcopy(sdata_blobs)
-        blob["table"].obs["region"] = "blobs_polygons"
-        blob["table"].uns["spatialdata_attrs"]["region"] = "blobs_polygons"
-        blob.shapes["blobs_polygons"]["value"] = [1, 2, 3, 4, 5]
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        blob.pl.render_shapes(
+    def test_plot_datashader_can_color_with_norm_no_clipping(self, sdata_blobs_shapes_annotated: SpatialData):
+        sdata_blobs_shapes_annotated.pl.render_shapes(
             element="blobs_polygons",
             color="value",
             norm=Normalize(2, 4, clip=False),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()
 
-    def test_plot_datashader_norm_vmin_eq_vmax_without_clip(self, sdata_blobs: SpatialData):
-        blob = deepcopy(sdata_blobs)
-        blob["table"].obs["region"] = "blobs_polygons"
-        blob["table"].uns["spatialdata_attrs"]["region"] = "blobs_polygons"
-        blob.shapes["blobs_polygons"]["value"] = [1, 2, 3, 4, 5]
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        blob.pl.render_shapes(
+    def test_plot_datashader_norm_vmin_eq_vmax_without_clip(self, sdata_blobs_shapes_annotated: SpatialData):
+        sdata_blobs_shapes_annotated.pl.render_shapes(
             element="blobs_polygons",
             color="value",
             norm=Normalize(3, 3, clip=False),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()
 
-    def test_plot_datashader_norm_vmin_eq_vmax_with_clip(self, sdata_blobs: SpatialData):
-        blob = deepcopy(sdata_blobs)
-        blob["table"].obs["region"] = "blobs_polygons"
-        blob["table"].uns["spatialdata_attrs"]["region"] = "blobs_polygons"
-        blob.shapes["blobs_polygons"]["value"] = [1, 2, 3, 4, 5]
-        cmap = matplotlib.colormaps["viridis"]
-        cmap.set_under("black")
-        cmap.set_over("grey")
-        blob.pl.render_shapes(
+    def test_plot_datashader_norm_vmin_eq_vmax_with_clip(self, sdata_blobs_shapes_annotated: SpatialData):
+        sdata_blobs_shapes_annotated.pl.render_shapes(
             element="blobs_polygons",
             color="value",
             norm=Normalize(3, 3, clip=True),
-            cmap=cmap,
+            cmap=_viridis_with_under_over(),
             method="datashader",
             datashader_reduction="max",
         ).pl.show()