refactor(render_params): RenderParams base class for the 5 *RenderParams (#720)

Author: timtreis

src/spatialdata_plot/pl/render_params.py

@@ -231,13 +231,28 @@ class ScalebarParams:
     scalebar_kwargs: Mapping[str, Any] = field(default_factory=dict)
 
 
-@dataclass
-class ShapesRenderParams:
+@dataclass(kw_only=True)
+class RenderParams:
+    """Fields shared by every ``*RenderParams``.
+
+    These four are the only fields with identical type and default across all five renderers.
+    ``kw_only=True`` keeps field order irrelevant across inheritance (subclasses add required fields
+    such as ``cmap_params``), and all call sites construct these dataclasses by keyword. Subclasses
+    carry their renderer-specific fields (including ``cmap_params``, whose type varies per renderer).
+    """
+
+    element: str
+    zorder: int = 0
+    colorbar: bool | str | None = "auto"
+    colorbar_params: dict[str, object] | None = None
+
+
+@dataclass(kw_only=True)
+class ShapesRenderParams(RenderParams):
     """Shapes render parameters."""
 
     cmap_params: CmapParams
     outline_params: OutlineParams
-    element: str
     color: Color | None = None
     col_for_color: str | None = None
     col_for_outline_color: str | None = None
@@ -249,27 +264,23 @@ class ShapesRenderParams:
     scale: float = 1.0
     transfunc: Callable[[float], float] | None = None
     method: str | None = None
-    zorder: int = 0
     table_name: str | None = None
     table_layer: str | None = None
     shape: Literal["circle", "hex", "visium_hex", "square"] | None = None
     # Fast mode: render each shape as a single dot at its centroid instead of its geometry.
     as_points: bool = False
     size: float = 1.0  # marker size for as_points (matplotlib scatter ``s``)
     ds_reduction: _DsReduction | None = None
-    colorbar: bool | str | None = "auto"
-    colorbar_params: dict[str, object] | None = None
     # Multi-panel color: when set, this render entry belongs to the panel identified by this
     # color key. ``None`` means the entry is shared across every panel (e.g. a background layer).
     panel_key: str | None = None
 
 
-@dataclass
-class PointsRenderParams:
+@dataclass(kw_only=True)
+class PointsRenderParams(RenderParams):
     """Points render parameters."""
 
     cmap_params: CmapParams
-    element: str
     color: Color | None = None
     col_for_color: str | None = None
     groups: str | list[str] | None = None
@@ -278,42 +289,34 @@ class PointsRenderParams:
     size: float = 1.0
     transfunc: Callable[[float], float] | None = None
     method: str | None = None
-    zorder: int = 0
     table_name: str | None = None
     table_layer: str | None = None
     ds_reduction: _DsReduction | None = None
-    colorbar: bool | str | None = "auto"
-    colorbar_params: dict[str, object] | None = None
     density: bool = False
     density_how: Literal["linear", "log", "cbrt", "eq_hist"] = "linear"
 
 
-@dataclass
-class ImageRenderParams:
+@dataclass(kw_only=True)
+class ImageRenderParams(RenderParams):
     """Image render parameters."""
 
     cmap_params: list[CmapParams] | CmapParams
-    element: str
     channel: list[str] | list[int] | int | str | None = None
     palette: ListedColormap | list[str] | None = None
     alpha: float = 1.0
     scale: str | None = None
-    zorder: int = 0
-    colorbar: bool | str | None = "auto"
-    colorbar_params: dict[str, object] | None = None
     transfunc: Callable[[np.ndarray], np.ndarray] | list[Callable[[np.ndarray], np.ndarray]] | None = None
     grayscale: bool = False
     channels_as_legend: bool = False
     method: Literal["matplotlib", "datashader"] | None = None
     ds_reduction: _ImageDsReduction | None = None
 
 
-@dataclass
-class LabelsRenderParams:
+@dataclass(kw_only=True)
+class LabelsRenderParams(RenderParams):
     """Labels render parameters."""
 
     cmap_params: CmapParams
-    element: str
     color: Color | None = None
     col_for_color: str | None = None
     col_for_outline_color: str | None = None
@@ -328,9 +331,6 @@ class LabelsRenderParams:
     table_name: str | None = None
     table_layer: str | None = None
     transfunc: Callable[[float], float] | None = None
-    zorder: int = 0
-    colorbar: bool | str | None = "auto"
-    colorbar_params: dict[str, object] | None = None
     # Fast mode: render each label as a single dot at its centroid instead of the mask.
     as_points: bool = False
     size: float = 1.0  # marker size for as_points (matplotlib scatter ``s``)
@@ -341,11 +341,10 @@ class LabelsRenderParams:
     panel_key: str | None = None
 
 
-@dataclass
-class GraphRenderParams:
+@dataclass(kw_only=True)
+class GraphRenderParams(RenderParams):
     """Graph render parameters."""
 
-    element: str
     connectivity_obsp_key: str = "spatial_connectivities"
     table_name: str | None = None
     color: Color | None = None
@@ -363,6 +362,3 @@ class GraphRenderParams:
     linestyle: str | Sequence[str] = "solid"
     rasterize: bool = True
     include_self_loops: bool = False
-    zorder: int = 0
-    colorbar: bool | str | None = "auto"
-    colorbar_params: dict[str, object] | None = None

tests/pl/test_render_params.py

@@ -0,0 +1,190 @@
+"""Structural tests for the ``*RenderParams`` dataclass hierarchy.
+
+Locks the ``RenderParams`` base-class refactor: the five renderers share one base holding only the
+universal fields, and each subclass must keep exactly the field set (and the universal defaults) it
+had before the reparent. A dropped field or a flipped default is invisible to the image-baseline
+suite (CI-only), so it is asserted directly here.
+"""
+
+import dataclasses
+
+import pytest
+
+from spatialdata_plot.pl.render_params import (
+    GraphRenderParams,
+    ImageRenderParams,
+    LabelsRenderParams,
+    PointsRenderParams,
+    RenderParams,
+    ShapesRenderParams,
+)
+
+ALL_SUBCLASSES = [
+    ShapesRenderParams,
+    PointsRenderParams,
+    ImageRenderParams,
+    LabelsRenderParams,
+    GraphRenderParams,
+]
+
+# The only fields with identical type+default across all five renderers; everything else lives on
+# the subclasses. ``element`` is required; the other three carry their canonical defaults.
+UNIVERSAL_FIELDS = {"element", "zorder", "colorbar", "colorbar_params"}
+
+# Frozen per-subclass field-name snapshot (captured from the pre-refactor dataclasses). The set must
+# stay identical through the reparent; adding/removing a renderer field is a deliberate change that
+# updates this map.
+EXPECTED_FIELD_NAMES = {
+    ShapesRenderParams: {
+        "cmap_params",
+        "outline_params",
+        "element",
+        "color",
+        "col_for_color",
+        "col_for_outline_color",
+        "outline_table_name",
+        "groups",
+        "palette",
+        "outline_alpha",
+        "fill_alpha",
+        "scale",
+        "transfunc",
+        "method",
+        "zorder",
+        "table_name",
+        "table_layer",
+        "shape",
+        "as_points",
+        "size",
+        "ds_reduction",
+        "colorbar",
+        "colorbar_params",
+        "panel_key",
+    },
+    PointsRenderParams: {
+        "cmap_params",
+        "element",
+        "color",
+        "col_for_color",
+        "groups",
+        "palette",
+        "alpha",
+        "size",
+        "transfunc",
+        "method",
+        "zorder",
+        "table_name",
+        "table_layer",
+        "ds_reduction",
+        "colorbar",
+        "colorbar_params",
+        "density",
+        "density_how",
+    },
+    ImageRenderParams: {
+        "cmap_params",
+        "element",
+        "channel",
+        "palette",
+        "alpha",
+        "scale",
+        "zorder",
+        "colorbar",
+        "colorbar_params",
+        "transfunc",
+        "grayscale",
+        "channels_as_legend",
+        "method",
+        "ds_reduction",
+    },
+    LabelsRenderParams: {
+        "cmap_params",
+        "element",
+        "color",
+        "col_for_color",
+        "col_for_outline_color",
+        "outline_table_name",
+        "groups",
+        "contour_px",
+        "palette",
+        "outline_alpha",
+        "outline_color",
+        "fill_alpha",
+        "scale",
+        "table_name",
+        "table_layer",
+        "transfunc",
+        "zorder",
+        "colorbar",
+        "colorbar_params",
+        "as_points",
+        "size",
+        "method",
+        "panel_key",
+    },
+    GraphRenderParams: {
+        "element",
+        "connectivity_obsp_key",
+        "table_name",
+        "color",
+        "obs_col",
+        "obsp_key",
+        "cmap_params",
+        "palette_map",
+        "na_color",
+        "color_source",
+        "groups",
+        "group_key",
+        "edge_width",
+        "edge_alpha",
+        "weight_key",
+        "linestyle",
+        "rasterize",
+        "include_self_loops",
+        "zorder",
+        "colorbar",
+        "colorbar_params",
+    },
+}
+
+
+@pytest.mark.parametrize("cls", ALL_SUBCLASSES)
+def test_is_renderparams_subclass(cls):
+    assert issubclass(cls, RenderParams)
+
+
+def test_base_holds_only_universal_fields():
+    assert {f.name for f in dataclasses.fields(RenderParams)} == UNIVERSAL_FIELDS
+
+
+def test_base_universal_defaults():
+    defaults = {f.name: f.default for f in dataclasses.fields(RenderParams)}
+    assert defaults["element"] is dataclasses.MISSING  # required
+    assert defaults["zorder"] == 0
+    assert defaults["colorbar"] == "auto"
+    assert defaults["colorbar_params"] is None
+
+
+@pytest.mark.parametrize("cls", ALL_SUBCLASSES)
+def test_field_names_preserved(cls):
+    assert {f.name for f in dataclasses.fields(cls)} == EXPECTED_FIELD_NAMES[cls]
+
+
+@pytest.mark.parametrize("cls", ALL_SUBCLASSES)
+def test_universal_fields_inherited_with_defaults(cls):
+    names = {f.name for f in dataclasses.fields(cls)}
+    assert names >= UNIVERSAL_FIELDS
+    defaults = {f.name: f.default for f in dataclasses.fields(cls)}
+    assert defaults["zorder"] == 0
+    assert defaults["colorbar"] == "auto"
+    assert defaults["colorbar_params"] is None
+
+
+def test_keyword_construction_roundtrip():
+    # All construction in basic.py is keyword-only; kw_only=True must not break it, and the inherited
+    # universal fields must round-trip through the subclass constructor.
+    p = GraphRenderParams(element="graph", zorder=3, colorbar=False)
+    assert p.element == "graph"
+    assert p.zorder == 3
+    assert p.colorbar is False
+    assert p.colorbar_params is None  # inherited default