From 3c70e116e03e51753fb8fecc7fbf502f0e5587ba Mon Sep 17 00:00:00 2001 From: selmanozleyen Date: Sun, 30 Aug 2026 19:52:20 +0200 Subject: [PATCH 01/26] replace two enums with literals Both were closed vocabularies of strings that callers write as strings, and neither earned the class it was declared as. `DetectTissueMethod` used `enum.auto()`, so its members carried no value anyone read; every use was an equality test, and a string argument was coerced through `DetectTissueMethod[method.upper()]` before any of them ran. It was never exported either, while its own docstring told callers to pass `DetectTissueMethod.OTSU` -- advice they could not follow. The `.upper()` in that coercion is why `method` is lowered before the new membership check: `"OTSU"` was accepted before and still is. `QCMetric` was a `StrEnum`, so its members already were their own strings and worked as registry keys unchanged. Its validation, though, was `isinstance(m, QCMetric)`, which is False for the plain string the member compares equal to: `qc_image(metrics="tenengrad")` raised while the docs advertised a `StrEnum`. Both now validate against `get_args`, which accepts what callers actually write. Two behaviour changes worth naming. An unknown metric now raises `ValueError` rather than `TypeError` -- it is a bad value, not a bad type, and the old class was chosen to complain about enum membership. And `QCMetric` leaves the module namespace entirely, not just `__all__`: a page rendering as nothing but `alias of Literal[...]` documents an argument rather than a type callers hold, and the names it listed now sit on the parameter that takes them, in both `im.qc_image` and its `pl` counterpart. `InputKind` stays an enum. It is internal and never crosses the public boundary. --- src/squidpy/experimental/im/__init__.py | 2 - src/squidpy/experimental/im/_detect_tissue.py | 44 +++++------ src/squidpy/experimental/im/_qc_image.py | 69 +++++++++-------- src/squidpy/experimental/im/_qc_metrics.py | 77 ++++++++++--------- src/squidpy/experimental/pl/_qc_image.py | 5 +- tests/experimental/test_qc_image.py | 32 ++++++-- 6 files changed, 124 insertions(+), 105 deletions(-) diff --git a/src/squidpy/experimental/im/__init__.py b/src/squidpy/experimental/im/__init__.py index 25814834e..0d93dbf35 100644 --- a/src/squidpy/experimental/im/__init__.py +++ b/src/squidpy/experimental/im/__init__.py @@ -4,7 +4,6 @@ from ._detect_tissue import detect_tissue from ._make_tiles import make_tiles, make_tiles_from_spots from ._qc_image import qc_image -from ._qc_metrics import QCMetric from ._stain import ( StainFit, estimate_white_point, @@ -12,7 +11,6 @@ ) __all__ = [ - "QCMetric", "StainFit", "calculate_image_features", "detect_tissue", diff --git a/src/squidpy/experimental/im/_detect_tissue.py b/src/squidpy/experimental/im/_detect_tissue.py index 1eb29b3a6..3e9c67812 100644 --- a/src/squidpy/experimental/im/_detect_tissue.py +++ b/src/squidpy/experimental/im/_detect_tissue.py @@ -1,8 +1,7 @@ from __future__ import annotations -import enum from collections.abc import Iterator, Mapping, Sequence -from typing import Any, Literal, cast +from typing import Any, Literal, cast, get_args import dask.array as da import numpy as np @@ -35,11 +34,8 @@ from ._utils import flatten_channels, get_element_data - -class DetectTissueMethod(enum.Enum): - OTSU = enum.auto() - FELZENSZWALB = enum.auto() - WEKA = enum.auto() +#: The segmentation methods :func:`detect_tissue` accepts. +DetectTissueMethod = Literal["otsu", "felzenszwalb", "weka"] #: Background flag per corner: ``(top-left, top-right, bottom-left, bottom-right)``. @@ -63,8 +59,8 @@ def _normalize_corners(corners_are_background: bool | Sequence[bool]) -> _Corner #: The params type each method takes. OTSU is absent: it accepts none. _METHOD_PARAMS: dict[DetectTissueMethod, type[FelzenszwalbParams | WekaParams]] = { - DetectTissueMethod.FELZENSZWALB: FelzenszwalbParams, - DetectTissueMethod.WEKA: WekaParams, + "felzenszwalb": FelzenszwalbParams, + "weka": WekaParams, } @@ -173,7 +169,7 @@ def detect_tissue( *, image_key: str, scale: str = "auto", - method: DetectTissueMethod | str = DetectTissueMethod.OTSU, + method: DetectTissueMethod = "otsu", method_params: FelzenszwalbParams | WekaParams | Mapping[str, Any] | None = None, channel_format: Literal["infer", "rgb", "rgba", "multichannel"] = "infer", corners_are_background: bool | Sequence[bool] = True, @@ -202,9 +198,9 @@ def detect_tissue( method Tissue detection method. Valid options are: - - `DetectTissueMethod.OTSU` or `"otsu"` - Otsu thresholding with background detection. - - `DetectTissueMethod.FELZENSZWALB` or `"felzenszwalb"` - Felzenszwalb superpixel segmentation. - - `DetectTissueMethod.WEKA` or `"weka"` - Trainable segmentation with corner background priors and RGB multiscale features. + - `"otsu"` - Otsu thresholding with background detection. + - `"felzenszwalb"` - Felzenszwalb superpixel segmentation. + - `"weka"` - Trainable segmentation with corner background priors and RGB multiscale features. method_params Optional parameters specific to the selected method. For `"felzenszwalb"`, provide a mapping of ``FelzenszwalbParams`` keys. For `"weka"`, provide a mapping of ``WekaParams`` @@ -264,22 +260,20 @@ def detect_tissue( Processing is performed at an appropriate resolution and then upscaled to match the original image dimensions. """ - # Normalize method - if isinstance(method, str): - try: - method = DetectTissueMethod[method.upper()] - except KeyError as e: - raise ValueError('method must be "otsu", "felzenszwalb", or "weka"') from e + # Case-insensitive, as the enum lookup it replaces was. + method = method.lower() if isinstance(method, str) else method + if method not in get_args(DetectTissueMethod): + raise ValueError(f"method must be one of {get_args(DetectTissueMethod)}, found {method!r}") logger.info(f"Detecting tissue with method: {method}") corners = _normalize_corners(corners_are_background) if not 0 < corner_size_pct <= 1: raise ValueError(f"`corner_size_pct` must be in (0, 1], got {corner_size_pct}.") - if method == DetectTissueMethod.WEKA and not any(corners): + if method == "weka" and not any(corners): raise ValueError("WEKA tissue detection requires corner background priors; set corners_are_background=True.") - if method == DetectTissueMethod.OTSU: + if method == "otsu": if method_params is not None: raise ValueError("`method_params` are not supported for OTSU tissue detection.") resolved_method_params = None @@ -306,7 +300,7 @@ def detect_tissue( # Channel flattening (greyscale) for threshold-based methods img_grey = None - if method != DetectTissueMethod.WEKA: + if method != "weka": img_grey_da: xr.DataArray = flatten_channels(img=img_src, channel_format=channel_format) if need_downscale: logger.info("Downscaling for faster computation.") @@ -315,7 +309,7 @@ def detect_tissue( img_grey = img_grey_da.values # may compute # Prepare color image for WEKA (keeps channels) - if method == DetectTissueMethod.WEKA: + if method == "weka": if need_downscale: logger.info("Downscaling for faster computation.") img_weka = _downscale_with_dask_multichannel(img_rgb=img_src, target_pixels=auto_max_pixels) @@ -332,10 +326,10 @@ def detect_tissue( ) # First-pass foreground - if method == DetectTissueMethod.OTSU: + if method == "otsu": img_fg_mask_bool = _segment_otsu(img_grey=img_grey, corners=corners, corner_size_pct=corner_size_pct) img_fg_mask_bool = _apply_border_margin(img_fg_mask_bool, normalized_margins) - elif method == DetectTissueMethod.WEKA: + elif method == "weka": wp = cast(WekaParams, resolved_method_params) img_fg_mask_bool = _segment_weka( img=img_weka, diff --git a/src/squidpy/experimental/im/_qc_image.py b/src/squidpy/experimental/im/_qc_image.py index f95966190..d9bea659a 100644 --- a/src/squidpy/experimental/im/_qc_image.py +++ b/src/squidpy/experimental/im/_qc_image.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections import defaultdict -from typing import Literal +from typing import Literal, get_args import dask import dask.array as da @@ -26,19 +26,19 @@ ) _DEFAULT_HNE_METRICS: list[QCMetric] = [ - QCMetric.TENENGRAD, - QCMetric.VAR_OF_LAPLACIAN, - QCMetric.ENTROPY, - QCMetric.BRIGHTNESS_MEAN, - QCMetric.HEMATOXYLIN_MEAN, - QCMetric.EOSIN_MEAN, + "tenengrad", + "var_of_laplacian", + "entropy", + "brightness_mean", + "hematoxylin_mean", + "eosin_mean", ] _DEFAULT_GENERIC_METRICS: list[QCMetric] = [ - QCMetric.TENENGRAD, - QCMetric.VAR_OF_LAPLACIAN, - QCMetric.ENTROPY, - QCMetric.BRIGHTNESS_MEAN, + "tenengrad", + "var_of_laplacian", + "entropy", + "brightness_mean", ] @@ -72,8 +72,16 @@ def qc_image( scale Scale level to use for processing. Defaults to ``"scale0"``. metrics - QC metrics to compute. Can be a single metric or list of metrics. - If ``None``, uses sensible defaults based on ``is_hne``. + QC metric name, or a list of them. ``None`` uses sensible defaults based on + ``is_hne``. Accepted values, by what they measure: + + - sharpness -- ``"tenengrad"``, ``"var_of_laplacian"``, ``"variance"``, + ``"fft_high_freq_energy"``, ``"haar_wavelet_energy"`` + - intensity -- ``"brightness_mean"``, ``"brightness_std"``, ``"entropy"`` + - staining, H&E only -- ``"hematoxylin_mean"``, ``"hematoxylin_std"``, + ``"eosin_mean"``, ``"eosin_std"``, ``"he_ratio"`` + - artifacts, H&E only -- ``"fold_fraction"`` + - tissue coverage -- ``"tissue_fraction"`` tile_size Size of tiles for analysis. If ``"auto"``, automatically determines size. is_hne @@ -116,20 +124,21 @@ def qc_image( if metrics is None: metrics = list(_DEFAULT_HNE_METRICS if is_hne else _DEFAULT_GENERIC_METRICS) - elif isinstance(metrics, QCMetric): + elif isinstance(metrics, str): metrics = [metrics] else: metrics = list(metrics) - if not isinstance(metrics, list) or not all(isinstance(m, QCMetric) for m in metrics): - available = ", ".join(m.value for m in QCMetric) - raise TypeError(f"metrics must be QCMetric or list of QCMetric. Available: {available}") + unknown = [m for m in metrics if m not in get_args(QCMetric)] + if unknown: + available = ", ".join(get_args(QCMetric)) + raise ValueError(f"Unknown metrics {unknown}. Available: {available}") # Validate H&E constraint if not is_hne: hne_requested = _HNE_METRICS & set(metrics) if hne_requested: - names = ", ".join(m.value for m in hne_requested) + names = ", ".join(hne_requested) raise ValueError( f"H&E-specific metrics ({names}) cannot be used when is_hne=False. " f"Set is_hne=True or remove these metrics." @@ -189,11 +198,11 @@ def qc_image( # Build all dask graphs lazily delayed_scores: dict[str, da.Array] = {} hed_metric_indices = { - QCMetric.HEMATOXYLIN_MEAN: 0, - QCMetric.HEMATOXYLIN_STD: 1, - QCMetric.EOSIN_MEAN: 2, - QCMetric.EOSIN_STD: 3, - QCMetric.HE_RATIO: 4, + "hematoxylin_mean": 0, + "hematoxylin_std": 1, + "eosin_mean": 2, + "eosin_std": 3, + "he_ratio": 4, } hed_scores: da.Array | None = None @@ -208,20 +217,18 @@ def qc_image( drop_axis=2, new_axis=2, ) - delayed_scores[m.value] = hed_scores[..., hed_metric_indices[m]] - logger.info(f"- Calculating metric: '{m.value}'") + delayed_scores[m] = hed_scores[..., hed_metric_indices[m]] + logger.info(f"- Calculating metric: '{m}'") continue kind, metric_func = get_metric_info(m) source = prepared_inputs[kind] if kind == InputKind.RGB: - delayed_scores[m.value] = da.map_blocks( - metric_func, source, dtype=np.float32, chunks=out_chunks, drop_axis=2 - ) + delayed_scores[m] = da.map_blocks(metric_func, source, dtype=np.float32, chunks=out_chunks, drop_axis=2) else: - delayed_scores[m.value] = da.map_blocks(metric_func, source, dtype=np.float32, chunks=out_chunks) - logger.info(f"- Calculating metric: '{m.value}'") + delayed_scores[m] = da.map_blocks(metric_func, source, dtype=np.float32, chunks=out_chunks) + logger.info(f"- Calculating metric: '{m}'") # Single dask.compute() across all metric types if progress: @@ -232,7 +239,7 @@ def qc_image( all_scores: dict[str, np.ndarray] = dict(zip(delayed_scores.keys(), results, strict=True)) # Build AnnData - metric_names = [m.value for m in metrics] + metric_names = list(metrics) first = next(iter(all_scores.values())) cents, _ = tg.centroids_and_polygons() n_tiles = first.size diff --git a/src/squidpy/experimental/im/_qc_metrics.py b/src/squidpy/experimental/im/_qc_metrics.py index 05c6567bc..abed1a488 100644 --- a/src/squidpy/experimental/im/_qc_metrics.py +++ b/src/squidpy/experimental/im/_qc_metrics.py @@ -2,6 +2,7 @@ from collections.abc import Callable from enum import StrEnum +from typing import Literal import numpy as np @@ -34,36 +35,38 @@ class InputKind(StrEnum): MASK = "mask" # (ty, tx) binary float32 -class QCMetric(StrEnum): +#: The metrics :func:`~squidpy.experimental.im.qc_image` can compute. +QCMetric = Literal[ # Sharpness (grayscale input) - TENENGRAD = "tenengrad" - VAR_OF_LAPLACIAN = "var_of_laplacian" - VARIANCE = "variance" - FFT_HIGH_FREQ_ENERGY = "fft_high_freq_energy" - HAAR_WAVELET_ENERGY = "haar_wavelet_energy" + "tenengrad", + "var_of_laplacian", + "variance", + "fft_high_freq_energy", + "haar_wavelet_energy", # Intensity (grayscale input) - BRIGHTNESS_MEAN = "brightness_mean" - BRIGHTNESS_STD = "brightness_std" - ENTROPY = "entropy" + "brightness_mean", + "brightness_std", + "entropy", # Staining (RGB input, H&E only) - HEMATOXYLIN_MEAN = "hematoxylin_mean" - HEMATOXYLIN_STD = "hematoxylin_std" - EOSIN_MEAN = "eosin_mean" - EOSIN_STD = "eosin_std" - HE_RATIO = "he_ratio" + "hematoxylin_mean", + "hematoxylin_std", + "eosin_mean", + "eosin_std", + "he_ratio", # Artifacts (RGB input, H&E only) - FOLD_FRACTION = "fold_fraction" + "fold_fraction", # Tissue coverage (mask input) - TISSUE_FRACTION = "tissue_fraction" + "tissue_fraction", +] _HNE_METRICS: set[QCMetric] = { - QCMetric.HEMATOXYLIN_MEAN, - QCMetric.HEMATOXYLIN_STD, - QCMetric.EOSIN_MEAN, - QCMetric.EOSIN_STD, - QCMetric.HE_RATIO, - QCMetric.FOLD_FRACTION, + "hematoxylin_mean", + "hematoxylin_std", + "eosin_mean", + "eosin_std", + "he_ratio", + "fold_fraction", } @@ -71,25 +74,25 @@ class QCMetric(StrEnum): _METRIC_REGISTRY: dict[QCMetric, tuple[InputKind, MetricFn]] = { # Sharpness (grayscale) - QCMetric.TENENGRAD: (InputKind.GRAYSCALE, tenengrad_mean), - QCMetric.VAR_OF_LAPLACIAN: (InputKind.GRAYSCALE, laplacian_variance), - QCMetric.VARIANCE: (InputKind.GRAYSCALE, pop_variance), - QCMetric.FFT_HIGH_FREQ_ENERGY: (InputKind.GRAYSCALE, fft_high_freq_energy), - QCMetric.HAAR_WAVELET_ENERGY: (InputKind.GRAYSCALE, haar_wavelet_energy), + "tenengrad": (InputKind.GRAYSCALE, tenengrad_mean), + "var_of_laplacian": (InputKind.GRAYSCALE, laplacian_variance), + "variance": (InputKind.GRAYSCALE, pop_variance), + "fft_high_freq_energy": (InputKind.GRAYSCALE, fft_high_freq_energy), + "haar_wavelet_energy": (InputKind.GRAYSCALE, haar_wavelet_energy), # Intensity (grayscale) - QCMetric.BRIGHTNESS_MEAN: (InputKind.GRAYSCALE, brightness_mean), - QCMetric.BRIGHTNESS_STD: (InputKind.GRAYSCALE, brightness_std), - QCMetric.ENTROPY: (InputKind.GRAYSCALE, entropy), + "brightness_mean": (InputKind.GRAYSCALE, brightness_mean), + "brightness_std": (InputKind.GRAYSCALE, brightness_std), + "entropy": (InputKind.GRAYSCALE, entropy), # Staining (RGB, H&E only) - QCMetric.HEMATOXYLIN_MEAN: (InputKind.RGB, hematoxylin_mean), - QCMetric.HEMATOXYLIN_STD: (InputKind.RGB, hematoxylin_std), - QCMetric.EOSIN_MEAN: (InputKind.RGB, eosin_mean), - QCMetric.EOSIN_STD: (InputKind.RGB, eosin_std), - QCMetric.HE_RATIO: (InputKind.RGB, he_ratio), + "hematoxylin_mean": (InputKind.RGB, hematoxylin_mean), + "hematoxylin_std": (InputKind.RGB, hematoxylin_std), + "eosin_mean": (InputKind.RGB, eosin_mean), + "eosin_std": (InputKind.RGB, eosin_std), + "he_ratio": (InputKind.RGB, he_ratio), # Artifacts (RGB, H&E only) - QCMetric.FOLD_FRACTION: (InputKind.RGB, fold_fraction), + "fold_fraction": (InputKind.RGB, fold_fraction), # Tissue coverage (mask) - QCMetric.TISSUE_FRACTION: (InputKind.MASK, tissue_fraction), + "tissue_fraction": (InputKind.MASK, tissue_fraction), } diff --git a/src/squidpy/experimental/pl/_qc_image.py b/src/squidpy/experimental/pl/_qc_image.py index 4a27d2341..d500e91c5 100644 --- a/src/squidpy/experimental/pl/_qc_image.py +++ b/src/squidpy/experimental/pl/_qc_image.py @@ -33,7 +33,7 @@ def qc_image( SpatialData object containing QC results. image_key : str Image key used in qc_image function. - metrics : QCMetric or list of QCMetric, optional + metrics : str or list of str, optional Specific metrics to plot. If None, plots all calculated metrics. figsize : tuple, optional Figure size (width, height). Auto-calculated if None. @@ -66,8 +66,7 @@ def qc_image( if metrics is not None: metrics_list = metrics if isinstance(metrics, list) else [metrics] metrics_to_plot = [] - for metric in metrics_list: - metric_name = metric.value if isinstance(metric, QCMetric) else metric + for metric_name in metrics_list: if metric_name not in calculated_metrics: raise ValueError(f"Metric '{metric_name}' not found. Available: {calculated_metrics}") metrics_to_plot.append(metric_name) diff --git a/tests/experimental/test_qc_image.py b/tests/experimental/test_qc_image.py index 1fd62cfe2..2894cddbd 100644 --- a/tests/experimental/test_qc_image.py +++ b/tests/experimental/test_qc_image.py @@ -33,6 +33,24 @@ def sdata_hne(_sdata_hne_with_tissue): return copy.deepcopy(_sdata_hne_with_tissue) +def test_qc_image_accepts_a_bare_metric_string(sdata_hne) -> None: + """A single metric name, not wrapped in a list. + + `QCMetric` was a `StrEnum`, but the guard was `isinstance(m, QCMetric)`, which is False + for the plain string the member compares equal to -- so this raised `TypeError` while + the docs advertised the string form. + """ + sq.experimental.im.qc_image(sdata_hne, image_key="hne", tile_size=_FAST_TILE, metrics="tenengrad", progress=False) + assert any("tenengrad" in name for name in sdata_hne.tables["qc_img_hne"].var_names) + + +def test_qc_image_names_the_unknown_metric(sdata_hne) -> None: + with pytest.raises(ValueError, match=r"Unknown metrics \['not_a_metric'\]"): + sq.experimental.im.qc_image( + sdata_hne, image_key="hne", tile_size=_FAST_TILE, metrics=["not_a_metric"], progress=False + ) + + class TestQCImage(PlotTester, metaclass=PlotTesterMeta): def test_plot_calc_qc_image_hne(self, sdata_hne): """Test QC image overlay with a single sharpness metric.""" @@ -40,7 +58,7 @@ def test_plot_calc_qc_image_hne(self, sdata_hne): sdata_hne, image_key="hne", tile_size=_FAST_TILE, - metrics=[sq.experimental.im.QCMetric.TENENGRAD], + metrics=["tenengrad"], progress=False, ) @@ -73,7 +91,7 @@ def test_plot_plot_qc_image(self, sdata_hne): sdata_hne, image_key="hne", tile_size=_FAST_TILE, - metrics=[sq.experimental.im.QCMetric.TENENGRAD], + metrics=["tenengrad"], progress=False, ) @@ -91,7 +109,7 @@ def test_qc_image_hne_metric_without_hne_flag(sdata_hne): image_key="hne", tile_size=_FAST_TILE, is_hne=False, - metrics=[sq.experimental.im.QCMetric.HEMATOXYLIN_MEAN], + metrics=["hematoxylin_mean"], ) @@ -144,7 +162,7 @@ def test_qc_image_rgb_metric(sdata_hne): image_key="hne", tile_size=_FAST_TILE, is_hne=True, - metrics=[sq.experimental.im.QCMetric.HEMATOXYLIN_MEAN], + metrics=["hematoxylin_mean"], detect_tissue=False, detect_outliers=False, progress=False, @@ -159,7 +177,7 @@ def test_qc_image_outlier_detection_with_tissue(sdata_hne): sdata_hne, image_key="hne", tile_size=_FAST_TILE, - metrics=[sq.experimental.im.QCMetric.TENENGRAD], + metrics=["tenengrad"], detect_outliers=True, detect_tissue=True, progress=False, @@ -180,7 +198,7 @@ def test_qc_image_outlier_detection_without_tissue(sdata_hne): sdata_hne, image_key="hne", tile_size=_FAST_TILE, - metrics=[sq.experimental.im.QCMetric.TENENGRAD], + metrics=["tenengrad"], detect_outliers=True, detect_tissue=False, progress=False, @@ -201,7 +219,7 @@ def test_qc_image_compute_only(sdata_hne): sdata_hne, image_key="hne", tile_size=_FAST_TILE, - metrics=[sq.experimental.im.QCMetric.TENENGRAD, sq.experimental.im.QCMetric.BRIGHTNESS_MEAN], + metrics=["tenengrad", "brightness_mean"], detect_outliers=False, detect_tissue=False, progress=False, From 43fa3097446099f7def2d3e2ce56ee9575bd70e9 Mon Sep 17 00:00:00 2001 From: selmanozleyen Date: Fri, 25 Sep 2026 21:38:43 +0200 Subject: [PATCH 02/26] Drop the double dashes from comments and docstrings --- src/squidpy/experimental/im/_qc_image.py | 10 +++++----- tests/experimental/test_qc_image.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/squidpy/experimental/im/_qc_image.py b/src/squidpy/experimental/im/_qc_image.py index d9bea659a..71c4befbd 100644 --- a/src/squidpy/experimental/im/_qc_image.py +++ b/src/squidpy/experimental/im/_qc_image.py @@ -75,13 +75,13 @@ def qc_image( QC metric name, or a list of them. ``None`` uses sensible defaults based on ``is_hne``. Accepted values, by what they measure: - - sharpness -- ``"tenengrad"``, ``"var_of_laplacian"``, ``"variance"``, + - sharpness: ``"tenengrad"``, ``"var_of_laplacian"``, ``"variance"``, ``"fft_high_freq_energy"``, ``"haar_wavelet_energy"`` - - intensity -- ``"brightness_mean"``, ``"brightness_std"``, ``"entropy"`` - - staining, H&E only -- ``"hematoxylin_mean"``, ``"hematoxylin_std"``, + - intensity: ``"brightness_mean"``, ``"brightness_std"``, ``"entropy"`` + - staining, H&E only: ``"hematoxylin_mean"``, ``"hematoxylin_std"``, ``"eosin_mean"``, ``"eosin_std"``, ``"he_ratio"`` - - artifacts, H&E only -- ``"fold_fraction"`` - - tissue coverage -- ``"tissue_fraction"`` + - artifacts, H&E only: ``"fold_fraction"`` + - tissue coverage: ``"tissue_fraction"`` tile_size Size of tiles for analysis. If ``"auto"``, automatically determines size. is_hne diff --git a/tests/experimental/test_qc_image.py b/tests/experimental/test_qc_image.py index 2894cddbd..dcf1ad9c8 100644 --- a/tests/experimental/test_qc_image.py +++ b/tests/experimental/test_qc_image.py @@ -37,7 +37,7 @@ def test_qc_image_accepts_a_bare_metric_string(sdata_hne) -> None: """A single metric name, not wrapped in a list. `QCMetric` was a `StrEnum`, but the guard was `isinstance(m, QCMetric)`, which is False - for the plain string the member compares equal to -- so this raised `TypeError` while + for the plain string the member compares equal to. So this raised `TypeError` while the docs advertised the string form. """ sq.experimental.im.qc_image(sdata_hne, image_key="hne", tile_size=_FAST_TILE, metrics="tenengrad", progress=False) From 23875980c01fd51be595dcb8a7e8052d26233e1e Mon Sep 17 00:00:00 2001 From: selmanozleyen Date: Wed, 30 Sep 2026 16:44:02 +0200 Subject: [PATCH 03/26] Accept any casing for string choices through one helper, make it a contribution rule --- docs/contributing.md | 1 + src/squidpy/_constants/_utils.py | 10 + src/squidpy/_validators.py | 17 +- src/squidpy/datasets/_datasets.py | 5 +- .../im/_calculate_image_features.py | 4 +- src/squidpy/experimental/im/_detect_tissue.py | 6 +- src/squidpy/experimental/im/_qc_image.py | 14 +- .../experimental/im/_stain/_normalize.py | 4 +- .../experimental/im/_stain/_reference.py | 5 +- src/squidpy/experimental/im/_utils.py | 20 +- src/squidpy/experimental/pl/_qc_image.py | 7 +- .../experimental/tl/_stitched_labels.py | 19 +- src/squidpy/gr/_clusterers.py | 7 +- src/squidpy/gr/_nhood.py | 11 +- src/squidpy/gr/_niche.py | 10 +- src/squidpy/gr/_ppatterns.py | 3 +- src/squidpy/pl/_graph.py | 16 +- tests/datasets/test_downloader.py | 4 +- tests/experimental/test_detect_tissue.py | 4 +- tests/experimental/test_qc_image.py | 13 +- .../test_stain_decompose_public.py | 2 +- tests/experimental/test_stain_normalize.py | 2 +- tests/experimental/test_stain_reference.py | 2 +- tests/experimental/test_stitched_labels.py | 8 +- tests/graph/test_nhood.py | 4 +- tests/graph/test_niche.py | 4 +- tests/test_string_choices.py | 173 ++++++++++++++++++ tests/test_validators.py | 18 +- 28 files changed, 303 insertions(+), 90 deletions(-) create mode 100644 tests/test_string_choices.py diff --git a/docs/contributing.md b/docs/contributing.md index 9509c7f1b..9c8bd62f4 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -171,6 +171,7 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors - **Keyword-only arguments**: a public function takes its data object (`adata`, `sdata` or `data`) positionally, and every other argument is keyword-only, e.g. `def nhood_enrichment(adata, *, cluster_key, ...)`. Avoid catch-all `**kwargs` that forward whatever they are given unchecked. - **Parameter bags**: a `*Params` {class}`~typing.TypedDict` in {mod}`squidpy.types`, each key declaring its default as `Annotated[type, Default(value)]`, only when the knobs depend on another argument (one bag per `method`, taken as `method_params`) or several functions share them (taken as `**kwargs: Unpack[XParams]`). Resolve it with `resolve_params` from `squidpy._params`, so unknown keys raise, and keep its keys out of the signature. +- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. diff --git a/src/squidpy/_constants/_utils.py b/src/squidpy/_constants/_utils.py index 7c0fa2feb..b9c051add 100644 --- a/src/squidpy/_constants/_utils.py +++ b/src/squidpy/_constants/_utils.py @@ -65,6 +65,16 @@ def __new__( # noqa: D102 class ModeEnum(ErrorFormatterABC, PrettyEnum, metaclass=ABCEnumMeta): """Enum which prints available values when invalid value has been passed.""" + @classmethod + def _missing_(cls, value: object) -> ModeEnum | None: + # String choices are case-insensitive; an unknown value still raises the formatted error. + from squidpy._validators import normalize_choice + + try: + return cls(normalize_choice(value, [m.value for m in cls], name=cls.__name__)) + except ValueError: + return None + @property def s(self) -> str: """Return the :attr:`value` as :class:`str`.""" diff --git a/src/squidpy/_validators.py b/src/squidpy/_validators.py index e83a6a748..b84b59ede 100644 --- a/src/squidpy/_validators.py +++ b/src/squidpy/_validators.py @@ -90,10 +90,19 @@ def assert_isinstance(value: Any, expected_type: type | tuple[type, ...], *, nam raise TypeError(f"Expected `{name}` to be of type `{type_names}`, got `{type(value).__name__}`.") -def assert_one_of(value: Any, options: Sequence[Any], *, name: str) -> None: - """Raise ValueError if *value* is not in *options*.""" - if value not in options: - raise ValueError(f"Expected `{name}` to be one of `{list(options)}`, got `{value!r}`.") +def normalize_choice(value: Any, options: Iterable[Any], *, name: str) -> Any: + """Return the option in *options* matching *value*, ignoring the case of strings. + + Raise ValueError naming the valid options if nothing matches. + """ + options = list(options) + if isinstance(value, str): + for opt in options: + if isinstance(opt, str) and opt.casefold() == value.casefold(): + return opt + elif value in options: + return value + raise ValueError(f"Expected `{name}` to be one of `{options}`, got `{value!r}`.") def assert_key_in(obj: Any, key: str, *, attr: str, obj_name: str, extra_msg: str = "") -> None: diff --git a/src/squidpy/datasets/_datasets.py b/src/squidpy/datasets/_datasets.py index 65e10025e..35da6ecb2 100644 --- a/src/squidpy/datasets/_datasets.py +++ b/src/squidpy/datasets/_datasets.py @@ -10,6 +10,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Literal +from squidpy._validators import normalize_choice from squidpy.datasets._downloader import download from squidpy.datasets._registry import dataset_names, get_registry from squidpy.read._utils import PathLike @@ -123,9 +124,7 @@ def visium( # guard against the visium_10x names specifically: a valid-but-wrong-type name # (e.g. "imc", an AnnData dataset) would otherwise pass and fail deep in the # anndata loader with a confusing ``unexpected keyword argument 'include_hires_tiff'``. - visium_samples = dataset_names("visium_10x") - if sample_id not in visium_samples: - raise ValueError(f"Unknown Visium sample: {sample_id}. Available samples: {visium_samples}") + sample_id = normalize_choice(sample_id, dataset_names("visium_10x"), name="sample_id") # downloads land in /visium_10x// return download(sample_id, base_dir, include_hires_tiff=include_hires_tiff) diff --git a/src/squidpy/experimental/im/_calculate_image_features.py b/src/squidpy/experimental/im/_calculate_image_features.py index 0f534e15e..2c4cb258f 100644 --- a/src/squidpy/experimental/im/_calculate_image_features.py +++ b/src/squidpy/experimental/im/_calculate_image_features.py @@ -26,6 +26,7 @@ from spatialdata.transformations import get_transformation from threadpoolctl import threadpool_limits +from squidpy._validators import normalize_choice from squidpy.experimental.im._tiling import ( CellInfo, _run_tiled, @@ -775,8 +776,7 @@ def _prepare_lazy( """ _validate_inputs(sdata, image_key=image_key, labels_key=labels_key, shapes_key=shapes_key, scale=scale) - if align_mode not in ("strict", "rasterize"): - raise ValueError(f"`align_mode` must be 'strict' or 'rasterize'; got {align_mode!r}.") + align_mode = normalize_choice(align_mode, ("strict", "rasterize"), name="align_mode") image_da = None if image_key is not None: diff --git a/src/squidpy/experimental/im/_detect_tissue.py b/src/squidpy/experimental/im/_detect_tissue.py index 3e9c67812..119163f9c 100644 --- a/src/squidpy/experimental/im/_detect_tissue.py +++ b/src/squidpy/experimental/im/_detect_tissue.py @@ -27,6 +27,7 @@ _yx_from_shape, legacy_random, ) +from squidpy._validators import normalize_choice from squidpy.types import ( FelzenszwalbParams, WekaParams, @@ -260,10 +261,7 @@ def detect_tissue( Processing is performed at an appropriate resolution and then upscaled to match the original image dimensions. """ - # Case-insensitive, as the enum lookup it replaces was. - method = method.lower() if isinstance(method, str) else method - if method not in get_args(DetectTissueMethod): - raise ValueError(f"method must be one of {get_args(DetectTissueMethod)}, found {method!r}") + method = normalize_choice(method, get_args(DetectTissueMethod), name="method") logger.info(f"Detecting tissue with method: {method}") diff --git a/src/squidpy/experimental/im/_qc_image.py b/src/squidpy/experimental/im/_qc_image.py index 71c4befbd..95f91d4d7 100644 --- a/src/squidpy/experimental/im/_qc_image.py +++ b/src/squidpy/experimental/im/_qc_image.py @@ -1,6 +1,7 @@ from __future__ import annotations from collections import defaultdict +from collections.abc import Iterable from typing import Literal, get_args import dask @@ -15,6 +16,7 @@ from spatialdata.models import TableModel from squidpy._utils import _ensure_dim_order +from squidpy._validators import normalize_choice from squidpy.experimental.im._intensity_metrics import hed_metrics from squidpy.experimental.im._qc_metrics import _HNE_METRICS, InputKind, QCMetric, get_metric_info from squidpy.experimental.im._utils import ( @@ -124,15 +126,21 @@ def qc_image( if metrics is None: metrics = list(_DEFAULT_HNE_METRICS if is_hne else _DEFAULT_GENERIC_METRICS) - elif isinstance(metrics, str): - metrics = [metrics] + elif isinstance(metrics, str) or not isinstance(metrics, Iterable): + metrics = [metrics] # a non-iterable then fails the name check below else: metrics = list(metrics) - unknown = [m for m in metrics if m not in get_args(QCMetric)] + unknown, resolved = [], [] + for m in metrics: + try: + resolved.append(normalize_choice(m, get_args(QCMetric), name="metrics")) + except ValueError: + unknown.append(m) if unknown: available = ", ".join(get_args(QCMetric)) raise ValueError(f"Unknown metrics {unknown}. Available: {available}") + metrics = resolved # Validate H&E constraint if not is_hne: diff --git a/src/squidpy/experimental/im/_stain/_normalize.py b/src/squidpy/experimental/im/_stain/_normalize.py index b6dc869bd..f2bdcfd2c 100644 --- a/src/squidpy/experimental/im/_stain/_normalize.py +++ b/src/squidpy/experimental/im/_stain/_normalize.py @@ -24,6 +24,7 @@ from squidpy._params import resolve_params from squidpy._utils import _get_scale_factors +from squidpy._validators import normalize_choice from squidpy.experimental.im._stain._constants import RUIFROK_HE from squidpy.experimental.im._stain._conversion import _check_channel_dim, cast_to_image_dtype from squidpy.experimental.im._stain._decomposition import ( @@ -275,8 +276,7 @@ def fit_stain_reference( ------- The fitted :class:`~squidpy.experimental.im.StainFit`. Nothing is written to ``sdata``. """ - if method not in _VALID_METHODS: - raise ValueError(f"Unknown method {method!r}; expected one of {list(_VALID_METHODS)}.") + method = normalize_choice(method, _VALID_METHODS, name="method") da = _resolve_image(sdata, image_key=image_key, scale=scale, prefer="coarsest") validate_rgb_range(da) params = resolve_params(method_params, _METHOD_PARAMS[method], validate=_METHOD_VALIDATORS[method]) diff --git a/src/squidpy/experimental/im/_stain/_reference.py b/src/squidpy/experimental/im/_stain/_reference.py index 8cdfa904f..7bff77219 100644 --- a/src/squidpy/experimental/im/_stain/_reference.py +++ b/src/squidpy/experimental/im/_stain/_reference.py @@ -11,6 +11,8 @@ import numpy as np +from squidpy._validators import normalize_choice + if TYPE_CHECKING: import spatialdata as sd import xarray as xr @@ -73,8 +75,7 @@ class StainFit: max_concentrations: np.ndarray | None = None def __post_init__(self) -> None: - if self.method not in _VALID_METHODS: - raise ValueError(f"Unknown method {self.method!r}; expected one of {sorted(_VALID_METHODS)}.") + object.__setattr__(self, "method", normalize_choice(self.method, sorted(_VALID_METHODS), name="method")) if self.method in _DECOMPOSITION_METHODS: if self.stain_matrix is None: diff --git a/src/squidpy/experimental/im/_utils.py b/src/squidpy/experimental/im/_utils.py index 9c1404ddd..63ee55c3f 100644 --- a/src/squidpy/experimental/im/_utils.py +++ b/src/squidpy/experimental/im/_utils.py @@ -13,6 +13,7 @@ from spatialdata.transformations import get_transformation, set_transformation from squidpy._utils import _yx_from_shape +from squidpy._validators import normalize_choice class TileGrid: @@ -190,6 +191,7 @@ def flatten_channels( xr.DataArray Greyscale image with shape (y, x) """ + channel_format = normalize_choice(channel_format, ("infer", "rgb", "rgba", "multichannel"), name="channel_format") n_channels = img.sizes["c"] # 1 channel: always return greyscale @@ -216,19 +218,13 @@ def flatten_channels( weights = xr.DataArray([0.299, 0.587, 0.114, 0.0], dims=["c"], coords={"c": img.coords["c"]}) return (img * weights).sum(dim="c") - elif channel_format == "infer": - if n_channels == 3: - # 3 channels + infer -> RGB luminance formula - weights = xr.DataArray([0.299, 0.587, 0.114], dims=["c"], coords={"c": img.coords["c"]}) - return (img * weights).sum(dim="c") - else: - # 2 channels or 4+ channels + infer -> multichannel - return img.mean(dim="c") - + elif n_channels == 3: + # 3 channels + infer -> RGB luminance formula + weights = xr.DataArray([0.299, 0.587, 0.114], dims=["c"], coords={"c": img.coords["c"]}) + return (img * weights).sum(dim="c") else: - raise ValueError( - f"Invalid channel_format: {channel_format}. Must be one of 'infer', 'rgb', 'rgba', 'multichannel'." - ) + # 2 channels or 4+ channels + infer -> multichannel + return img.mean(dim="c") def get_mask_dask(sdata: SpatialData, *, mask_key: str, scale: str) -> da.Array: diff --git a/src/squidpy/experimental/pl/_qc_image.py b/src/squidpy/experimental/pl/_qc_image.py index d500e91c5..56fbb97a6 100644 --- a/src/squidpy/experimental/pl/_qc_image.py +++ b/src/squidpy/experimental/pl/_qc_image.py @@ -8,6 +8,7 @@ from spatialdata import SpatialData from spatialdata._logging import logger as logg +from squidpy._validators import normalize_choice from squidpy.experimental.im._qc_metrics import QCMetric @@ -65,11 +66,7 @@ def qc_image( if metrics is not None: metrics_list = metrics if isinstance(metrics, list) else [metrics] - metrics_to_plot = [] - for metric_name in metrics_list: - if metric_name not in calculated_metrics: - raise ValueError(f"Metric '{metric_name}' not found. Available: {calculated_metrics}") - metrics_to_plot.append(metric_name) + metrics_to_plot = [normalize_choice(m, calculated_metrics, name="metrics") for m in metrics_list] else: metrics_to_plot = calculated_metrics diff --git a/src/squidpy/experimental/tl/_stitched_labels.py b/src/squidpy/experimental/tl/_stitched_labels.py index 3a224cff6..cddd3b0d6 100644 --- a/src/squidpy/experimental/tl/_stitched_labels.py +++ b/src/squidpy/experimental/tl/_stitched_labels.py @@ -24,6 +24,7 @@ from spatialdata.models import Labels2DModel, TableModel from spatialdata.transformations import get_transformation +from squidpy._validators import normalize_choice from squidpy.experimental.utils._labels import resolve_labels_array __all__ = ["make_stitched_labels"] @@ -186,14 +187,14 @@ def _close_block(block: np.ndarray) -> np.ndarray: ) -def _resolve_strategy(strategy: str | Callable[[pd.Series], object]) -> Callable[[pd.Series], object]: +def _check_strategy(strategy: str | Callable[[pd.Series], object]) -> str | Callable[[pd.Series], object]: + """Return a callable unchanged, or the built-in strategy name matching *strategy*.""" if callable(strategy): return strategy - if strategy not in _BUILTIN_STRATEGIES: - raise ValueError( - f"Unknown merge_strategy {strategy!r}. Use one of {sorted(_BUILTIN_STRATEGIES)} or pass a callable." - ) - return _BUILTIN_STRATEGIES[strategy] + try: + return normalize_choice(strategy, _BUILTIN_STRATEGIES, name="merge_strategy") + except ValueError as e: + raise ValueError(f"{e} Or pass a callable.") from None # Strategies whose result is always one of the input values, so an integer @@ -254,7 +255,7 @@ def _aggregate_X( # General path: bounded per-group work, sparse-preserving output. reducer = _BUILTIN_X_REDUCERS[strategy] if isinstance(strategy, str) else None - strategy_fn = None if reducer is not None else _resolve_strategy(strategy) + strategy_fn = None if reducer is not None else (strategy if callable(strategy) else _BUILTIN_STRATEGIES[strategy]) Xc = X.tocsr() if sparse_in else np.asarray(X) out = sp.lil_matrix((n_groups, n_cols), dtype=out_dtype) if sparse_in else np.zeros((n_groups, n_cols), out_dtype) for i, idx in enumerate(group_indices): @@ -320,7 +321,7 @@ def _collapse_groups( if "label_id" not in obs.columns: raise ValueError("AnnData missing 'label_id'.") - _resolve_strategy(merge_strategy) # validate strategy name early + merge_strategy = _check_strategy(merge_strategy) group_ids = obs["stitch_group_id"].astype(int).to_numpy() # Positional indices per group in one linear pass (sorted by group id), # instead of an O(n_cells * n_groups) per-group np.where scan. @@ -492,7 +493,7 @@ def make_stitched_labels( ) # Validate merge_strategy up front so an invalid value fails fast even when # write_table=False (the aggregation that would otherwise raise is skipped). - _resolve_strategy(merge_strategy) + merge_strategy = _check_strategy(merge_strategy) # table_key_added implies the user wants a table; reject the contradictory combo # rather than silently ignoring the key. if table_key_added is not None and not write_table: diff --git a/src/squidpy/gr/_clusterers.py b/src/squidpy/gr/_clusterers.py index aba9f2167..6d0bcfe41 100644 --- a/src/squidpy/gr/_clusterers.py +++ b/src/squidpy/gr/_clusterers.py @@ -8,6 +8,8 @@ from fast_array_utils.types import HasArrayNamespace as Array from sklearn.base import BaseEstimator, ClusterMixin +from squidpy._validators import normalize_choice + @runtime_checkable class Clusterer(Protocol): @@ -62,10 +64,11 @@ def fit(self, X: Array, y: None = None) -> LeidenClusterer: # Default to the igraph backend so niche labels are reproducible across versions; # leidenalg is deprecated in scanpy and unstable on small graphs. See # scverse/squidpy#1260. - kwargs: dict[str, Any] = {"flavor": self.flavor, "n_iterations": self.n_iterations} + flavor = normalize_choice(self.flavor, ("igraph", "leidenalg"), name="flavor") + kwargs: dict[str, Any] = {"flavor": flavor, "n_iterations": self.n_iterations} # scanpy's igraph backend only supports undirected graphs and errors if `directed` # is left at the leidenalg default of True, so pin it to False. - if self.flavor == "igraph": + if flavor == "igraph": kwargs["directed"] = False sc.tl.leiden( shell, diff --git a/src/squidpy/gr/_nhood.py b/src/squidpy/gr/_nhood.py index 6c1b78c8e..411ecd397 100644 --- a/src/squidpy/gr/_nhood.py +++ b/src/squidpy/gr/_nhood.py @@ -41,7 +41,7 @@ numba_threads, parallelize, ) -from squidpy._validators import assert_key_in_adata, assert_positive +from squidpy._validators import assert_key_in_adata, assert_positive, normalize_choice from squidpy.gr._utils import ( _assert_categorical_obs, _assert_connectivity_key, @@ -383,10 +383,8 @@ def nhood_enrichment( _assert_connectivity_key(adata, key=connectivity_key) assert_positive(n_perms, name="n_perms") - if normalization not in _NORM_CODES: - raise ValueError(f"Invalid normalization mode `{normalization}`. Choose from {sorted(_NORM_CODES)}.") - if handle_nan not in ("keep", "zero"): - raise ValueError(f"Invalid `handle_nan` mode `{handle_nan}`. Choose from 'keep', 'zero'.") + normalization = normalize_choice(normalization, _NORM_CODES, name="normalization") + handle_nan = normalize_choice(handle_nan, ("keep", "zero"), name="handle_nan") adj = adata.obsp[connectivity_key] if not issparse(adj): @@ -1067,8 +1065,7 @@ def nhood_aggregate( reach counts once: a cell two paths away is still one cell. """ _assert_hop_request(adata, connectivity_key=connectivity_key, hops=hops) - if aggregation not in ("mean", "sum", "variance"): - raise ValueError(f"'aggregation' must be 'mean', 'sum' or 'variance', got {aggregation!r}") + aggregation = normalize_choice(aggregation, ("mean", "sum", "variance"), name="aggregation") weights = [1.0] * len(hops) if hop_weights is None else list(hop_weights) if len(weights) != len(hops): raise ValueError(f"'hop_weights' has {len(weights)} value(s) but there are {len(hops)} hop(s)") diff --git a/src/squidpy/gr/_niche.py b/src/squidpy/gr/_niche.py index ef0222b91..3b2309481 100644 --- a/src/squidpy/gr/_niche.py +++ b/src/squidpy/gr/_niche.py @@ -27,7 +27,7 @@ deprecated_randomness_param, legacy_random, ) -from squidpy._validators import assert_isinstance, assert_key_in_adata, assert_one_of +from squidpy._validators import assert_isinstance, assert_key_in_adata, normalize_choice from squidpy.gr._clusterers import Clusterer, LeidenClusterer from squidpy.gr._nhood import ( _aggregate_over, @@ -195,6 +195,7 @@ def calculate_niche( # noqa: PLR0917, deprecated and removed in v1.9.0 stacklevel=3, ) + flavor = normalize_choice(flavor, ("neighborhood", "utag", "cellcharter", "spatialleiden"), name="flavor") # cellcharter-only defaults stay guarded: filling them for other flavors would trip # the "not used for flavor" warning in _check_unnecessary_args if mask is not None and flavor == "spatialleiden": @@ -927,7 +928,7 @@ def _validate_niche_args( # noqa: PLR0917, deprecated and removed in v1.9.0 """ assert_isinstance(data, (AnnData, SpatialData), name="data") - assert_one_of(flavor, ["neighborhood", "utag", "cellcharter", "spatialleiden"], name="flavor") + normalize_choice(flavor, ["neighborhood", "utag", "cellcharter", "spatialleiden"], name="flavor") if isinstance(data, SpatialData) and table_key is None: raise TypeError("missing required keyword-only argument: 'table_key'") @@ -1051,7 +1052,7 @@ def _validate_niche_args( # noqa: PLR0917, deprecated and removed in v1.9.0 if aggregation is not None: assert_isinstance(aggregation, str, name="aggregation") - assert_one_of(aggregation, ["mean", "variance"], name="aggregation") + normalize_choice(aggregation, ["mean", "variance"], name="aggregation") assert_isinstance(n_components, int, name="n_components") if n_components < 1: @@ -1202,8 +1203,7 @@ def _nhop_pca_embedding( n_jobs: int | None = None, ) -> Array: """Reduced features and their ring aggregates, concatenated as in CellCharter.""" - if aggregation not in ("mean", "variance"): - raise ValueError(f"'aggregation' must be 'mean' or 'variance', got {aggregation!r}") + aggregation = normalize_choice(aggregation, ("mean", "variance"), name="aggregation") if distance < 1: raise ValueError(f"'distance' must be >= 1, got {distance}") diff --git a/src/squidpy/gr/_ppatterns.py b/src/squidpy/gr/_ppatterns.py index e14d1e17d..3008c1fa6 100644 --- a/src/squidpy/gr/_ppatterns.py +++ b/src/squidpy/gr/_ppatterns.py @@ -34,7 +34,7 @@ get_n_processes, parallelize, ) -from squidpy._validators import assert_key_in_adata, assert_positive +from squidpy._validators import assert_key_in_adata, assert_positive, normalize_choice from squidpy.gr._utils import ( _assert_categorical_obs, _assert_connectivity_key, @@ -207,6 +207,7 @@ def extract_obsm(adata: AnnData, ixs: int | Sequence[int] | None) -> tuple[NDArr return adata.obsm[layer][:, ixs].T, ixs + attr = normalize_choice(attr, ("obs", "X", "obsm"), name="attr") if attr == "X": vals, index = extract_X(adata, genes) # type: ignore elif attr == "obs": diff --git a/src/squidpy/pl/_graph.py b/src/squidpy/pl/_graph.py index ad43bcccc..dcca7d8bb 100644 --- a/src/squidpy/pl/_graph.py +++ b/src/squidpy/pl/_graph.py @@ -20,7 +20,7 @@ from squidpy._constants._constants import RipleyStat from squidpy._constants._pkg_constants import Key from squidpy._docs import d -from squidpy._validators import assert_non_empty_sequence, get_valid_values +from squidpy._validators import assert_non_empty_sequence, get_valid_values, normalize_choice from squidpy.gr._utils import _assert_categorical_obs from squidpy.pl._color_utils import Palette_t, _get_palette, _maybe_set_colors from squidpy.pl._utils import _heatmap, save_fig @@ -101,7 +101,15 @@ def centrality_scores( score = scores if score is None else score score = assert_non_empty_sequence(score, name="centrality scores") - score = sorted(get_valid_values(score, scores)) + + def _canonical(s: str) -> str: + # Case-insensitive match; unknown names pass through and are dropped by get_valid_values. + try: + return normalize_choice(s, scores, name="score") + except ValueError: + return s + + score = sorted(get_valid_values([_canonical(s) for s in score], scores)) fig, axs = plt.subplots(1, len(score), figsize=figsize, dpi=dpi, constrained_layout=True) axs = np.ravel(axs) # make into iterable @@ -244,6 +252,7 @@ def nhood_enrichment( %(plotting_returns)s """ _assert_categorical_obs(adata, key=cluster_key) + mode = normalize_choice(mode, ("zscore", "count"), name="mode") array = _get_data(adata, cluster_key=cluster_key, func_name="nhood_enrichment")[mode] ad = AnnData(X=array, obs={cluster_key: pd.Categorical(adata.obs[cluster_key].cat.categories)}) @@ -422,11 +431,10 @@ def ripley( """ _assert_categorical_obs(adata, key=cluster_key) - res = _get_data(adata, cluster_key=cluster_key, func_name="ripley", mode=mode) - mode = RipleyStat(mode) # type: ignore[assignment] if TYPE_CHECKING: assert isinstance(mode, RipleyStat) + res = _get_data(adata, cluster_key=cluster_key, func_name="ripley", mode=mode.s) legend_kwargs = dict(legend_kwargs) if "loc" not in legend_kwargs: diff --git a/tests/datasets/test_downloader.py b/tests/datasets/test_downloader.py index 40b2b8c85..62988dc69 100644 --- a/tests/datasets/test_downloader.py +++ b/tests/datasets/test_downloader.py @@ -29,13 +29,13 @@ class TestVisiumValidation: """Offline validation of the ``visium()`` sample-name guard (no network).""" def test_unknown_sample_raises(self): - with pytest.raises(ValueError, match="Unknown Visium sample"): + with pytest.raises(ValueError, match="Expected `sample_id` to be one of"): visium("definitely_not_a_sample") def test_valid_but_wrong_type_rejected(self): # `imc` is a valid registry entry but an AnnData dataset, not a Visium 10x # sample. It must be rejected before any download is attempted. - with pytest.raises(ValueError, match="Unknown Visium sample"): + with pytest.raises(ValueError, match="Expected `sample_id` to be one of"): visium("imc") diff --git a/tests/experimental/test_detect_tissue.py b/tests/experimental/test_detect_tissue.py index 4c4a0d0f6..6d37dcd70 100644 --- a/tests/experimental/test_detect_tissue.py +++ b/tests/experimental/test_detect_tissue.py @@ -168,8 +168,10 @@ def test_weka_border_margin_is_not_a_params_key() -> None: ({"corners_are_background": (True, False)}, ValueError, "sequence of 4 bools"), ({"corners_are_background": np.ones((2, 2), dtype=bool)}, ValueError, "sequence of 4 bools"), ({"corner_size_pct": 0.0}, ValueError, "`corner_size_pct` must be in"), + ({"method": "bogus"}, ValueError, "Expected `method` to be one of"), + ({"method": None}, ValueError, "Expected `method` to be one of"), ], - ids=["string", "wrong_length", "not_flat", "zero_corner_size"], + ids=["string", "wrong_length", "not_flat", "zero_corner_size", "unknown_method", "none_method"], ) def test_invalid_corners_raise(sdata_hne, kwargs, error, match) -> None: with pytest.raises(error, match=match): diff --git a/tests/experimental/test_qc_image.py b/tests/experimental/test_qc_image.py index dcf1ad9c8..3bd508a45 100644 --- a/tests/experimental/test_qc_image.py +++ b/tests/experimental/test_qc_image.py @@ -44,11 +44,14 @@ def test_qc_image_accepts_a_bare_metric_string(sdata_hne) -> None: assert any("tenengrad" in name for name in sdata_hne.tables["qc_img_hne"].var_names) -def test_qc_image_names_the_unknown_metric(sdata_hne) -> None: - with pytest.raises(ValueError, match=r"Unknown metrics \['not_a_metric'\]"): - sq.experimental.im.qc_image( - sdata_hne, image_key="hne", tile_size=_FAST_TILE, metrics=["not_a_metric"], progress=False - ) +@pytest.mark.parametrize( + ("metrics", "match"), + [(["not_a_metric"], r"Unknown metrics \['not_a_metric'\]"), (5, r"Unknown metrics \[5\]\. Available")], + ids=["unknown_name", "not_iterable"], +) +def test_qc_image_names_the_unknown_metric(sdata_hne, metrics, match) -> None: + with pytest.raises(ValueError, match=match): + sq.experimental.im.qc_image(sdata_hne, image_key="hne", tile_size=_FAST_TILE, metrics=metrics, progress=False) class TestQCImage(PlotTester, metaclass=PlotTesterMeta): diff --git a/tests/experimental/test_stain_decompose_public.py b/tests/experimental/test_stain_decompose_public.py index db3aac371..e709e9b50 100644 --- a/tests/experimental/test_stain_decompose_public.py +++ b/tests/experimental/test_stain_decompose_public.py @@ -146,7 +146,7 @@ def test_explicit_background_is_used(self) -> None: class TestUnknownMethod: def test_fit_unknown_method_raises(self) -> None: sdata = _make_sdata(_synthetic_rgb()) - with pytest.raises(ValueError, match="Unknown method"): + with pytest.raises(ValueError, match="Expected `method` to be one of"): fit_stain_reference(sdata, image_key="img", method="bogus") diff --git a/tests/experimental/test_stain_normalize.py b/tests/experimental/test_stain_normalize.py index ae24a14f6..afbf3f708 100644 --- a/tests/experimental/test_stain_normalize.py +++ b/tests/experimental/test_stain_normalize.py @@ -57,7 +57,7 @@ def test_missing_image_key_raises(self, rgb_values: np.ndarray) -> None: def test_unknown_method_raises(self, rgb_values: np.ndarray) -> None: sdata = _make_sdata(rgb_values) - with pytest.raises(ValueError, match="Unknown method"): + with pytest.raises(ValueError, match="Expected `method` to be one of"): fit_stain_reference(sdata, image_key="img", method="bogus") def test_rgba_image_rejected(self) -> None: diff --git a/tests/experimental/test_stain_reference.py b/tests/experimental/test_stain_reference.py index 7197d1ca4..68ed309e4 100644 --- a/tests/experimental/test_stain_reference.py +++ b/tests/experimental/test_stain_reference.py @@ -37,7 +37,7 @@ def test_reinhard_basic() -> None: def test_unknown_method_raises() -> None: - with pytest.raises(ValueError, match="Unknown method"): + with pytest.raises(ValueError, match="Expected `method` to be one of"): StainFit(method="not-a-method") # type: ignore[arg-type] diff --git a/tests/experimental/test_stitched_labels.py b/tests/experimental/test_stitched_labels.py index 119a94a01..ce890a2f3 100644 --- a/tests/experimental/test_stitched_labels.py +++ b/tests/experimental/test_stitched_labels.py @@ -220,7 +220,11 @@ def test_aggregated_table_label_id_matches_new_element_ids(self, sdata_tile_boun [ ("qc_only", {"labels_key": "labels"}, "stitch_group_id"), ("qc_and_stitch", {"labels_key": "bogus"}, "not found"), - ("qc_and_stitch", {"labels_key": "labels", "merge_strategy": "bogus"}, "Unknown merge_strategy"), + ( + "qc_and_stitch", + {"labels_key": "labels", "merge_strategy": "bogus"}, + "Expected `merge_strategy` to be one of.*Or pass a callable", + ), ( "qc_and_stitch", {"labels_key": "labels", "write_table": False, "table_key_added": "x"}, @@ -461,7 +465,7 @@ def test_invalid_merge_strategy_raises_without_table(self, sdata_tile_boundary): """M9: merge_strategy is validated eagerly even when write_table=False.""" sdata, _ = sdata_tile_boundary _qc_and_stitch(sdata) - with pytest.raises(ValueError, match="Unknown merge_strategy"): + with pytest.raises(ValueError, match="Expected `merge_strategy` to be one of.*Or pass a callable"): sq.experimental.tl.make_stitched_labels( sdata, labels_key="labels", write_table=False, merge_strategy="bogus" ) diff --git a/tests/graph/test_nhood.py b/tests/graph/test_nhood.py index d4d8eb4ca..7e27d3611 100644 --- a/tests/graph/test_nhood.py +++ b/tests/graph/test_nhood.py @@ -295,8 +295,8 @@ def _one_cluster(adata: AnnData) -> None: @pytest.mark.parametrize( ("mutate", "kwargs", "match"), [ - (None, {"handle_nan": "nonsense"}, "Invalid `handle_nan` mode"), - (None, {"normalization": "invalid_mode"}, "Invalid normalization mode"), + (None, {"handle_nan": "nonsense"}, "Expected `handle_nan` to be one of"), + (None, {"normalization": "invalid_mode"}, "Expected `normalization` to be one of"), (_nan_cluster, {}, "Found `NaN` values"), (_one_cluster, {}, "Expected at least `2` clusters"), ], diff --git a/tests/graph/test_niche.py b/tests/graph/test_niche.py index 259cf8e91..923d689f3 100644 --- a/tests/graph/test_niche.py +++ b/tests/graph/test_niche.py @@ -461,8 +461,8 @@ def test_hop_adjacency_accepts_an_array_like(): ("kwargs", "match"), [ # hop 0 returns the features unaggregated, so it never reaches the per-hop check - ({"hops": (0,), "aggregation": "median"}, r"'aggregation' must be"), - ({"hops": (1,), "aggregation": "median"}, r"'aggregation' must be"), + ({"hops": (0,), "aggregation": "median"}, r"Expected `aggregation` to be one of"), + ({"hops": (1,), "aggregation": "median"}, r"Expected `aggregation` to be one of"), ({"hops": (1, 2), "hop_weights": [1.0]}, r"'hop_weights' has 1 value"), # averaging over a zero total used to return an all-NaN matrix silently ({"hops": (1, 2), "hop_weights": [1.0, -1.0], "aggregation": "mean"}, r"must not sum to zero"), diff --git a/tests/test_string_choices.py b/tests/test_string_choices.py new file mode 100644 index 000000000..e4f4d343f --- /dev/null +++ b/tests/test_string_choices.py @@ -0,0 +1,173 @@ +"""String choices are case-insensitive, and an unknown one names the valid options.""" + +from __future__ import annotations + +import numpy as np +import pytest +import spatialdata as sd +from spatialdata.models import Image2DModel, Labels2DModel + +import squidpy as sq +from squidpy.experimental.im import StainFit +from squidpy.experimental.im._utils import flatten_channels +from squidpy.gr._clusterers import LeidenClusterer + + +def _rgb_sdata() -> sd.SpatialData: + values = np.random.default_rng(0).uniform(40, 200, size=(3, 64, 64)).astype(np.uint8) + sdata = sd.SpatialData(images={"img": Image2DModel.parse(values, dims=("c", "y", "x"))}) + sdata.labels["img_tissue"] = Labels2DModel.parse(np.ones((64, 64), dtype=np.uint32), dims=("y", "x")) + return sdata + + +def _visium(monkeypatch: pytest.MonkeyPatch, sample_id: str) -> str: + monkeypatch.setattr("squidpy.datasets._datasets.download", lambda name, *_, **__: name) + return sq.datasets.visium(sample_id) + + +def _pl_nhood(adata, mode): + sq.gr.nhood_enrichment(adata, cluster_key="leiden", n_perms=2, rng=0) + sq.pl.nhood_enrichment(adata, cluster_key="leiden", mode=mode) + + +def _pl_qc_image(_, metric): + sdata = _rgb_sdata() + sq.experimental.im.qc_image( + sdata, image_key="img", metrics=["tenengrad"], tile_size=(32, 32), tissue_mask_key="img_tissue", progress=False + ) + sq.experimental.pl.qc_image(sdata, image_key="img", metrics=metric) + + +def _pl_ripley(adata, mode): + sq.gr.ripley(adata, cluster_key="leiden", mode="G", n_simulations=2, n_steps=5, rng=0) + sq.pl.ripley(adata, cluster_key="leiden", mode=mode) + + +# (data factory name, call(data, value), upper-case spelling, error name in the message) +_CASES = { + "gr.nhood_enrichment-normalization": ( + "nhood_data", + lambda a, v: sq.gr.nhood_enrichment(a, cluster_key="leiden", normalization=v, n_perms=2, rng=0, copy=True), + "TOTAL", + "`normalization`", + ), + "gr.nhood_enrichment-handle_nan": ( + "nhood_data", + lambda a, v: sq.gr.nhood_enrichment(a, cluster_key="leiden", handle_nan=v, n_perms=2, rng=0, copy=True), + "ZERO", + "`handle_nan`", + ), + "gr.spatial_autocorr-attr": ( + "nhood_data", + lambda a, v: sq.gr.spatial_autocorr(a, attr=v, genes=a.var_names[:2], copy=True), + "x", + "`attr`", + ), + "gr.spatial_autocorr-mode": ( + "nhood_data", + lambda a, v: sq.gr.spatial_autocorr(a, mode=v, genes=a.var_names[:2], copy=True), + "GEARY", + "`SpatialAutocorr`", + ), + "gr.ripley-mode": ( + "nhood_data", + lambda a, v: sq.gr.ripley(a, cluster_key="leiden", mode=v, n_simulations=2, n_steps=5, rng=0, copy=True), + "l", + "`RipleyStat`", + ), + "gr.spatial_neighbors-coord_type": ( + "nhood_data", + lambda a, v: sq.gr.spatial_neighbors(a, coord_type=v, copy=True), + "GENERIC", + "`CoordType`", + ), + "gr.spatial_neighbors_knn-transform": ( + "nhood_data", + lambda a, v: sq.gr.spatial_neighbors_knn(a, transform=v, copy=True), + "COSINE", + "`Transform`", + ), + "gr.calculate_niche_cellcharter-aggregation": ( + "dummy_adata2", + lambda a, v: sq.gr.calculate_niche_cellcharter(a, distance=2, aggregation=v, rng=0, copy=True), + "VARIANCE", + "`aggregation`", + ), + "gr.LeidenClusterer-flavor": ( + None, + lambda _, v: LeidenClusterer(flavor=v, n_neighbors=3).fit(np.random.default_rng(0).normal(size=(20, 3))), + "IGRAPH", + "`flavor`", + ), + "pl.nhood_enrichment-mode": ("nhood_data", _pl_nhood, "COUNT", "`mode`"), + "pl.ripley-mode": ("nhood_data", _pl_ripley, "g", "`RipleyStat`"), + "experimental.pl.qc_image-metrics": (None, _pl_qc_image, "TENENGRAD", "`metrics`"), + "datasets.visium-sample_id": ( + "monkeypatch", + _visium, + "V1_HUMAN_HEART", + "`sample_id`", + ), + "experimental.im.detect_tissue-method": ( + None, + lambda _, v: sq.experimental.im.detect_tissue(_rgb_sdata(), image_key="img", method=v, inplace=False), + "OTSU", + "`method`", + ), + "experimental.im.detect_tissue-channel_format": ( + None, + lambda _, v: flatten_channels(img=_rgb_sdata().images["img"], channel_format=v), + "RGB", + "`channel_format`", + ), + "experimental.im.qc_image-metrics": ( + None, + lambda _, v: sq.experimental.im.qc_image( + _rgb_sdata(), image_key="img", metrics=[v], tile_size=(32, 32), tissue_mask_key="img_tissue", progress=False + ), + "TENENGRAD", + "Unknown metrics", + ), + "experimental.im.calculate_image_features-align_mode": ( + None, + lambda _, v: sq.experimental.im.calculate_image_features( + _rgb_sdata(), + image_key="img", + labels_key="img_tissue", + features=["skimage:morphology"], + align_mode=v, + inplace=False, + ), + "RASTERIZE", + "`align_mode`", + ), + "experimental.im.fit_stain_reference-method": ( + None, + lambda _, v: sq.experimental.im.fit_stain_reference(_rgb_sdata(), image_key="img", method=v), + "REINHARD", + "`method`", + ), + "experimental.im.StainFit-method": ( + None, + lambda _, v: StainFit(method=v, mu=np.zeros(3), sigma=np.ones(3)), + "Reinhard", + "`method`", + ), +} + + +@pytest.mark.parametrize("case", list(_CASES), ids=list(_CASES)) +def test_string_choice_is_case_insensitive(request: pytest.FixtureRequest, case: str) -> None: + data_name, call, upper, name = _CASES[case] + data = None if data_name is None else request.getfixturevalue(data_name) + call(data, upper) + with pytest.raises(ValueError, match=f"{name}.*bogus|bogus.*{name}"): + call(data, "bogus") + + +def test_pl_centrality_scores_score_is_case_insensitive(nhood_data) -> None: + # Unknown names are dropped, not raised on (as before), so this is outside the table above. + sq.gr.centrality_scores(nhood_data, cluster_key="leiden") + sq.pl.centrality_scores(nhood_data, cluster_key="leiden", score="DEGREE_CENTRALITY") + with pytest.raises(ValueError, match="No valid values"): + sq.pl.centrality_scores(nhood_data, cluster_key="leiden", score="bogus") diff --git a/tests/test_validators.py b/tests/test_validators.py index ba3423484..ffcbf6810 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -13,10 +13,10 @@ assert_key_in_sdata, assert_non_empty_sequence, assert_non_negative, - assert_one_of, assert_positive, check_tuple_needles, get_valid_values, + normalize_choice, ) @@ -132,14 +132,16 @@ def test_wrong_type_tuple(self): assert_isinstance(3.14, (str, int), name="x") -# assert_one_of -class TestAssertOneOf: - def test_valid(self): - assert_one_of("a", ["a", "b", "c"], name="x") +# normalize_choice +class TestNormalizeChoice: + @pytest.mark.parametrize(("value", "expected"), [("a", "a"), ("A", "a"), ("x", "X"), (None, None), (4, 4)]) + def test_valid(self, value, expected): + assert normalize_choice(value, ["a", "X", None, 4], name="x") == expected - def test_invalid(self): - with pytest.raises(ValueError, match="one of"): - assert_one_of("z", ["a", "b"], name="x") + @pytest.mark.parametrize("value", ["z", 5, ["a"]]) + def test_invalid(self, value): + with pytest.raises(ValueError, match=r"Expected `x` to be one of `\['a', 'b'\]`"): + normalize_choice(value, ["a", "b"], name="x") # assert_key_in_adata From 9ee5bbb876d2f2ddb8d4467de665e4f24f10a5cf Mon Sep 17 00:00:00 2001 From: selmanozleyen Date: Thu, 1 Oct 2026 14:10:50 +0200 Subject: [PATCH 04/26] Drop the repeated flavor check and the unreachable aggregation raise --- src/squidpy/gr/_nhood.py | 8 +++----- src/squidpy/gr/_niche.py | 2 -- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/squidpy/gr/_nhood.py b/src/squidpy/gr/_nhood.py index 411ecd397..cee21caf8 100644 --- a/src/squidpy/gr/_nhood.py +++ b/src/squidpy/gr/_nhood.py @@ -1032,11 +1032,9 @@ def mean_over(x: Array | CSBase) -> Array | CSBase: if aggregation == "mean": return mean_over(features) - if aggregation == "variance": - mean = to_dense(mean_over(features)) - dense = to_dense(features) - return to_dense(mean_over(dense * dense)) - mean * mean - raise ValueError(f"'aggregation' must be 'mean', 'sum' or 'variance', got {aggregation!r}") + mean = to_dense(mean_over(features)) # "variance": both callers normalize `aggregation` + dense = to_dense(features) + return to_dense(mean_over(dense * dense)) - mean * mean def _assert_hop_request(adata: AnnData, *, connectivity_key: str, hops: Sequence[int]) -> None: diff --git a/src/squidpy/gr/_niche.py b/src/squidpy/gr/_niche.py index 3b2309481..c27210972 100644 --- a/src/squidpy/gr/_niche.py +++ b/src/squidpy/gr/_niche.py @@ -928,8 +928,6 @@ def _validate_niche_args( # noqa: PLR0917, deprecated and removed in v1.9.0 """ assert_isinstance(data, (AnnData, SpatialData), name="data") - normalize_choice(flavor, ["neighborhood", "utag", "cellcharter", "spatialleiden"], name="flavor") - if isinstance(data, SpatialData) and table_key is None: raise TypeError("missing required keyword-only argument: 'table_key'") From 6a01614c691754df8fc35bff7cc9673d0a1d035b Mon Sep 17 00:00:00 2001 From: selmanozleyen Date: Thu, 1 Oct 2026 14:57:52 +0200 Subject: [PATCH 05/26] Keep the aggregation raise, test a spelling the fall-through would not hide --- src/squidpy/gr/_nhood.py | 8 +++++--- tests/test_string_choices.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/squidpy/gr/_nhood.py b/src/squidpy/gr/_nhood.py index cee21caf8..411ecd397 100644 --- a/src/squidpy/gr/_nhood.py +++ b/src/squidpy/gr/_nhood.py @@ -1032,9 +1032,11 @@ def mean_over(x: Array | CSBase) -> Array | CSBase: if aggregation == "mean": return mean_over(features) - mean = to_dense(mean_over(features)) # "variance": both callers normalize `aggregation` - dense = to_dense(features) - return to_dense(mean_over(dense * dense)) - mean * mean + if aggregation == "variance": + mean = to_dense(mean_over(features)) + dense = to_dense(features) + return to_dense(mean_over(dense * dense)) - mean * mean + raise ValueError(f"'aggregation' must be 'mean', 'sum' or 'variance', got {aggregation!r}") def _assert_hop_request(adata: AnnData, *, connectivity_key: str, hops: Sequence[int]) -> None: diff --git a/tests/test_string_choices.py b/tests/test_string_choices.py index e4f4d343f..3ff9a02be 100644 --- a/tests/test_string_choices.py +++ b/tests/test_string_choices.py @@ -90,7 +90,7 @@ def _pl_ripley(adata, mode): "gr.calculate_niche_cellcharter-aggregation": ( "dummy_adata2", lambda a, v: sq.gr.calculate_niche_cellcharter(a, distance=2, aggregation=v, rng=0, copy=True), - "VARIANCE", + "MEAN", "`aggregation`", ), "gr.LeidenClusterer-flavor": ( From 496396805be68cbd667b473a6de9d6e7b98e063b Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 19:22:33 +0200 Subject: [PATCH 06/26] Replace five enum args with Literal spatial_autocorr(mode), ripley(mode) in gr and pl, spatial_neighbors(coord_type) and the transform argument on every builder now take plain string literals, resolved with normalize_choice. Drops ModeEnum._missing_, which only existed to give those enum-backed arguments the same case-insensitivity. normalize_choice unwraps Enum members, so CoordType.GRID and Transform.COSINE keep working for anyone who read them off the released signatures. --- docs/conf.py | 2 - src/squidpy/_constants/_constants.py | 19 ------- src/squidpy/_constants/_utils.py | 10 ---- src/squidpy/_validators.py | 3 ++ src/squidpy/gr/_build.py | 76 ++++++++++++++-------------- src/squidpy/gr/_ppatterns.py | 29 +++++------ src/squidpy/gr/_ripley.py | 34 ++++++------- src/squidpy/gr/neighbors.py | 42 ++++++++------- src/squidpy/pl/_graph.py | 13 ++--- tests/graph/test_ripley.py | 19 ++++--- tests/test_string_choices.py | 17 +++++-- 11 files changed, 122 insertions(+), 142 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 4d404bc51..a42908d7c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -147,8 +147,6 @@ ("py:class", "np.number"), ("py:class", "csr_matrix"), # no idea why those aren’t exported - ("py:class", "squidpy._constants._constants.SpatialAutocorr"), - ("py:class", "squidpy._constants._constants.CoordType"), ("py:class", "squidpy._constants._constants.Transform"), ("py:class", "pandas.core.frame.DataFrame"), ] diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index f88e41cb6..f5c40d141 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -36,12 +36,6 @@ class Transform(ModeEnum): NONE = None -@unique -class CoordType(ModeEnum): - GRID = "grid" - GENERIC = "generic" - - @unique class Processing(ModeEnum): SMOOTH = "smooth" @@ -90,12 +84,6 @@ class Symbol(ModeEnum): SQUARE = "square" -@unique -class SpatialAutocorr(ModeEnum): - MORAN = "moran" - GEARY = "geary" - - @unique class InferDimensions(ModeEnum): DEFAULT = "default" @@ -103,13 +91,6 @@ class InferDimensions(ModeEnum): Z_LAST = "z_last" -@unique -class RipleyStat(ModeEnum): - F = "F" - G = "G" - L = "L" - - @unique class ScatterShape(str, ModeEnum): CIRCLE = "circle" diff --git a/src/squidpy/_constants/_utils.py b/src/squidpy/_constants/_utils.py index b9c051add..7c0fa2feb 100644 --- a/src/squidpy/_constants/_utils.py +++ b/src/squidpy/_constants/_utils.py @@ -65,16 +65,6 @@ def __new__( # noqa: D102 class ModeEnum(ErrorFormatterABC, PrettyEnum, metaclass=ABCEnumMeta): """Enum which prints available values when invalid value has been passed.""" - @classmethod - def _missing_(cls, value: object) -> ModeEnum | None: - # String choices are case-insensitive; an unknown value still raises the formatted error. - from squidpy._validators import normalize_choice - - try: - return cls(normalize_choice(value, [m.value for m in cls], name=cls.__name__)) - except ValueError: - return None - @property def s(self) -> str: """Return the :attr:`value` as :class:`str`.""" diff --git a/src/squidpy/_validators.py b/src/squidpy/_validators.py index b84b59ede..af39afef7 100644 --- a/src/squidpy/_validators.py +++ b/src/squidpy/_validators.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Hashable, Iterable, Sequence +from enum import Enum from typing import TYPE_CHECKING, Any from squidpy._utils import _unique_order_preserving @@ -96,6 +97,8 @@ def normalize_choice(value: Any, options: Iterable[Any], *, name: str) -> Any: Raise ValueError naming the valid options if nothing matches. """ options = list(options) + if isinstance(value, Enum): + value = value.value if isinstance(value, str): for opt in options: if isinstance(opt, str) and opt.casefold() == value.casefold(): diff --git a/src/squidpy/gr/_build.py b/src/squidpy/gr/_build.py index 613852b4e..1f6e5a7db 100644 --- a/src/squidpy/gr/_build.py +++ b/src/squidpy/gr/_build.py @@ -3,7 +3,7 @@ from __future__ import annotations import warnings -from typing import Any, NamedTuple +from typing import Any, Literal, NamedTuple import geopandas as gpd import numpy as np @@ -24,11 +24,10 @@ ) from squidpy._compat import SKIP_OWN_FRAMES, old_positionals -from squidpy._constants._constants import CoordType, Transform from squidpy._constants._pkg_constants import Key -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import NDArrayA, get_n_numba_threads, thread_map -from squidpy._validators import assert_positive +from squidpy._validators import assert_positive, normalize_choice from squidpy.gr._utils import ( _assert_categorical_obs, _assert_spatial_basis, @@ -42,6 +41,8 @@ GridBuilder, KNNBuilder, RadiusBuilder, + TransformLike, + _as_transform, ) __all__ = [ @@ -64,13 +65,13 @@ class SpatialNeighborsResult[GraphMatrixT](NamedTuple): def _resolve_graph_builder( *, - coord_type: str | CoordType | None, + coord_type: Literal["grid", "generic"] | None, n_neighs: int | None, radius: float | tuple[float, float] | None, delaunay: bool | None, n_rings: int | None, percentile: float | None, - transform: str | Transform | None, + transform: TransformLike, set_diag: bool | None, has_spatial_uns: bool = False, ) -> GraphBuilder[Any, Any]: @@ -83,20 +84,20 @@ def _resolve_graph_builder( assert_positive(n_rings, name="n_rings") assert_positive(n_neighs, name="n_neighs") - transform = Transform.NONE if transform is None else Transform(transform) + transform = _as_transform(transform) if coord_type is None: if radius is not None: logg.warning( "Graph creation with `radius` is only available for generic coordinates. " f"Ignoring parameter `radius = {radius}`." ) - coord_type = CoordType.GRID if has_spatial_uns else CoordType.GENERIC + coord_type = "grid" if has_spatial_uns else "generic" else: - coord_type = CoordType(coord_type) + coord_type = normalize_choice(coord_type, ("grid", "generic"), name="coord_type") common: dict[str, Any] = {"transform": transform, "set_diag": set_diag} - if coord_type == CoordType.GRID: + if coord_type == "grid": if percentile is not None: raise ValueError( "`percentile` is not supported for grid coordinates. It only applies to generic (non-grid) graphs." @@ -130,7 +131,6 @@ def _resolve_graph_builder( @d.dedent -@inject_docs(t=Transform, c=CoordType) @old_positionals( "spatial_key", "elements_to_coordinate_systems", @@ -155,13 +155,13 @@ def spatial_neighbors( elements_to_coordinate_systems: dict[str, str] | None = None, table_key: str | None = None, library_key: str | None = None, - coord_type: str | CoordType | None = None, + coord_type: Literal["grid", "generic"] | None = None, n_neighs: int | None = None, radius: float | tuple[float, float] | None = None, delaunay: bool | None = None, n_rings: int | None = None, percentile: float | None = None, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, key_added: str = "spatial", copy: bool = False, @@ -197,38 +197,38 @@ def spatial_neighbors( coord_type Type of coordinate system. Valid options are: - - `{c.GRID.s!r}` - grid coordinates. - - `{c.GENERIC.s!r}` - generic coordinates. - - `None` - `{c.GRID.s!r}` if ``spatial_key`` is in :attr:`anndata.AnnData.uns` - with ``n_neighs = 6`` (Visium), otherwise use `{c.GENERIC.s!r}`. + - `'grid'` - grid coordinates. + - `'generic'` - generic coordinates. + - `None` - `'grid'` if ``spatial_key`` is in :attr:`anndata.AnnData.uns` + with ``n_neighs = 6`` (Visium), otherwise use `'generic'`. n_neighs Depending on the ``coord_type``: - - `{c.GRID.s!r}` - number of neighboring tiles. - - `{c.GENERIC.s!r}` - number of neighborhoods for non-grid data. Only used when ``delaunay = False``. + - `'grid'` - number of neighboring tiles. + - `'generic'` - number of neighborhoods for non-grid data. Only used when ``delaunay = False``. Defaults to ``6``. radius - Only available when ``coord_type = {c.GENERIC.s!r}``. + Only available when ``coord_type = 'generic'``. Depending on the type: - :class:`float` - compute the graph based on neighborhood radius. - :class:`tuple` - prune the final graph to only contain edges in interval `[min(radius), max(radius)]`. delaunay - Whether to compute the graph from Delaunay triangulation. Only used when ``coord_type = {c.GENERIC.s!r}``. + Whether to compute the graph from Delaunay triangulation. Only used when ``coord_type = 'generic'``. Defaults to ``False``. n_rings - Number of rings of neighbors for grid data. Only used when ``coord_type = {c.GRID.s!r}``. + Number of rings of neighbors for grid data. Only used when ``coord_type = 'grid'``. Defaults to ``1``. percentile - Percentile of the distances to use as threshold. Only used when ``coord_type = {c.GENERIC.s!r}``. + Percentile of the distances to use as threshold. Only used when ``coord_type = 'generic'``. transform Type of adjacency matrix transform. Valid options are: - - `{t.SPECTRAL.s!r}` - spectral transformation of the adjacency matrix. - - `{t.COSINE.s!r}` - cosine transformation of the adjacency matrix. - - `{t.NONE.v}` - no transformation of the adjacency matrix. + - `'spectral'` - spectral transformation of the adjacency matrix. + - `'cosine'` - cosine transformation of the adjacency matrix. + - `None` - no transformation of the adjacency matrix. set_diag Whether to set the diagonal of the spatial connectivities to `1.0`. key_added @@ -288,7 +288,7 @@ def spatial_neighbors( Grid-specific behavior ---------------------- Grid mode currently does not validate ``n_neighs`` to a fixed set - such as ``{{4, 6}}``. Internally it first queries the + such as ``{4, 6}``. Internally it first queries the ``n_neighs`` nearest candidates and then applies a distance-based correction tuned for grid-like coordinates. As a result: @@ -306,9 +306,9 @@ def spatial_neighbors( Otherwise, modifies the ``adata`` with the following keys: - - :attr:`anndata.AnnData.obsp` ``['{{key_added}}_connectivities']`` - the spatial connectivities. - - :attr:`anndata.AnnData.obsp` ``['{{key_added}}_distances']`` - the spatial distances. - - :attr:`anndata.AnnData.uns` ``['{{key_added}}']`` - :class:`dict` containing parameters. + - :attr:`anndata.AnnData.obsp` ``['{key_added}_connectivities']`` - the spatial connectivities. + - :attr:`anndata.AnnData.obsp` ``['{key_added}_distances']`` - the spatial distances. + - :attr:`anndata.AnnData.uns` ``['{key_added}']`` - :class:`dict` containing parameters. """ warnings.warn( "Calling `spatial_neighbors` is deprecated and will be removed in squidpy " @@ -521,7 +521,7 @@ def spatial_neighbors_knn( library_key: str | None = None, n_neighs: int = 6, percentile: float | None = None, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, key_added: str = "spatial", copy: bool = False, @@ -555,7 +555,7 @@ def spatial_neighbors_knn( spatial_neighbors_from_builder : Use :class:`~squidpy.gr.neighbors.KNNBuilder` directly for advanced customization. squidpy.gr.neighbors.KNNBuilder : k-nearest-neighbor builder class. """ - transform_enum = Transform.NONE if transform is None else Transform(transform) + transform_enum = _as_transform(transform) builder = KNNBuilder( n_neighs=n_neighs, percentile=percentile, @@ -591,7 +591,7 @@ def spatial_neighbors_radius( table_key: str | None = None, library_key: str | None = None, percentile: float | None = None, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, key_added: str = "spatial", copy: bool = False, @@ -628,7 +628,7 @@ def spatial_neighbors_radius( spatial_neighbors_from_builder : Use :class:`~squidpy.gr.neighbors.RadiusBuilder` directly for advanced customization. squidpy.gr.neighbors.RadiusBuilder : radius-based builder class. """ - transform_enum = Transform.NONE if transform is None else Transform(transform) + transform_enum = _as_transform(transform) builder = RadiusBuilder( radius=radius, percentile=percentile, @@ -664,7 +664,7 @@ def spatial_neighbors_delaunay( library_key: str | None = None, radius: float | tuple[float, float] | None = None, percentile: float | None = None, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, key_added: str = "spatial", copy: bool = False, @@ -705,7 +705,7 @@ def spatial_neighbors_delaunay( spatial_neighbors_from_builder : Use :class:`~squidpy.gr.neighbors.DelaunayBuilder` directly for advanced customization. squidpy.gr.neighbors.DelaunayBuilder : Delaunay triangulation builder class. """ - transform_enum = Transform.NONE if transform is None else Transform(transform) + transform_enum = _as_transform(transform) builder = DelaunayBuilder( radius=radius, percentile=percentile, @@ -742,7 +742,7 @@ def spatial_neighbors_grid( n_neighs: int = 6, n_rings: int = 1, delaunay: bool = False, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, key_added: str = "spatial", copy: bool = False, @@ -794,7 +794,7 @@ def spatial_neighbors_grid( """ assert_positive(n_rings, name="n_rings") assert_positive(n_neighs, name="n_neighs") - transform_enum = Transform.NONE if transform is None else Transform(transform) + transform_enum = _as_transform(transform) builder = GridBuilder( n_neighs=n_neighs, n_rings=n_rings, diff --git a/src/squidpy/gr/_ppatterns.py b/src/squidpy/gr/_ppatterns.py index 3b1e9d6ef..922ad3739 100644 --- a/src/squidpy/gr/_ppatterns.py +++ b/src/squidpy/gr/_ppatterns.py @@ -20,7 +20,6 @@ from statsmodels.stats.multitest import multipletests from squidpy._compat import old_positionals -from squidpy._constants._constants import SpatialAutocorr from squidpy._constants._pkg_constants import Key from squidpy._docs import d, inject_docs from squidpy._utils import ( @@ -55,7 +54,7 @@ @d.dedent -@inject_docs(key=Key.obsp.spatial_conn(), sp=SpatialAutocorr) +@inject_docs(key=Key.obsp.spatial_conn()) @old_positionals( "connectivity_key", "genes", @@ -79,7 +78,7 @@ def spatial_autocorr( *, connectivity_key: str = Key.obsp.spatial_conn(), genes: str | int | Sequence[str] | Sequence[int] | None = None, - mode: SpatialAutocorr | Literal["moran", "geary"] = "moran", + mode: Literal["moran", "geary"] = "moran", transformation: bool = True, n_perms: int | None = None, two_tailed: bool = False, @@ -129,8 +128,8 @@ def spatial_autocorr( mode Mode of score calculation: - - `{sp.MORAN.s!r}` - `Moran's I autocorrelation `_. - - `{sp.GEARY.s!r}` - `Geary's C autocorrelation `_. + - `'moran'` - `Moran's I autocorrelation `_. + - `'geary'` - `Geary's C autocorrelation `_. transformation If `True`, weights in :attr:`anndata.AnnData.obsp` ``['{key}']`` are row-normalized, @@ -168,8 +167,8 @@ def spatial_autocorr( Otherwise, modifies the ``adata`` with the following key: - - :attr:`anndata.AnnData.uns` ``['moranI']`` - the above mentioned dataframe, if ``mode = {sp.MORAN.s!r}``. - - :attr:`anndata.AnnData.uns` ``['gearyC']`` - the above mentioned dataframe, if ``mode = {sp.GEARY.s!r}``. + - :attr:`anndata.AnnData.uns` ``['moranI']`` - the above mentioned dataframe, if ``mode = 'moran'``. + - :attr:`anndata.AnnData.uns` ``['gearyC']`` - the above mentioned dataframe, if ``mode = 'geary'``. """ adata = extract_adata_if_sdata(adata, table_key=table_key) _assert_connectivity_key(adata, key=connectivity_key) @@ -217,15 +216,15 @@ def extract_obsm(adata: AnnData, ixs: int | Sequence[int] | None) -> tuple[NDArr else: raise NotImplementedError(f"Extracting from `adata.{attr}` is not yet implemented.") - mode = SpatialAutocorr(mode) - params = {"mode": mode.s, "transformation": transformation, "two_tailed": two_tailed} + mode = normalize_choice(mode, ("moran", "geary"), name="mode") + params = {"mode": mode, "transformation": transformation, "two_tailed": two_tailed} - if mode == SpatialAutocorr.MORAN: + if mode == "moran": params["func"] = morans_i params["stat"] = "I" params["expected"] = -1.0 / (adata.shape[0] - 1) # expected score params["ascending"] = False - elif mode == SpatialAutocorr.GEARY: + elif mode == "geary": params["func"] = gearys_c params["stat"] = "C" params["expected"] = 1.0 @@ -281,7 +280,7 @@ def extract_obsm(adata: AnnData, ixs: int | Sequence[int] | None) -> tuple[NDArr def _score_helper( perms: Sequence[int], - mode: SpatialAutocorr, + mode: Literal["moran", "geary"], g: spmatrix, vals: NDArrayA, rngs: Sequence[np.random.Generator], @@ -289,7 +288,7 @@ def _score_helper( queue: SigQueue | None = None, ) -> pd.DataFrame: score_perms = np.empty((len(perms), vals.shape[0])) - func = morans_i if mode == SpatialAutocorr.MORAN else gearys_c + func = morans_i if mode == "moran" else gearys_c for i, p in enumerate(perms): rng = rngs[p] @@ -538,13 +537,13 @@ def _analytic_pval(score: NDArrayA, g: spmatrix | NDArrayA, params: dict[str, An s02 = s0 * s0 match params["mode"]: - case SpatialAutocorr.GEARY.s: + case "geary": # Geary's C and Moran's I have different sampling variances under the # normality assumption (Cliff & Ord 1981). Use the Geary's C variance # (matching pysal/esda ``Geary``); reusing Moran's variance here gives a # miscalibrated analytic p-value (see #1183). Vscore_norm = ((2 * s1 + s2) * (n - 1) - 4 * s02) / (2 * (n + 1) * s02) - case SpatialAutocorr.MORAN.s: + case "moran": # Moran's I normality variance (Cliff & Ord 1981; pysal/esda ``Moran``). n2 = n * n v_num = n2 * s1 - n * s2 + 3 * s02 diff --git a/src/squidpy/gr/_ripley.py b/src/squidpy/gr/_ripley.py index 4c5ebcc05..0048310ab 100644 --- a/src/squidpy/gr/_ripley.py +++ b/src/squidpy/gr/_ripley.py @@ -14,17 +14,17 @@ from spatialdata import SpatialData from squidpy._compat import old_positionals -from squidpy._constants._constants import RipleyStat from squidpy._constants._pkg_constants import Key from squidpy._docs import d, inject_docs from squidpy._utils import NDArrayA, RNGLike, SeedLike, deprecated_randomness_param +from squidpy._validators import normalize_choice from squidpy.gr._utils import _assert_categorical_obs, _assert_spatial_basis, _save_data, extract_adata_if_sdata __all__ = ["ripley"] @d.dedent -@inject_docs(key=Key.obsm.spatial, rp=RipleyStat) +@inject_docs(key=Key.obsm.spatial) @old_positionals( "cluster_key", "mode", @@ -63,9 +63,9 @@ def ripley( %(rng_versionchanged)s According to the `'mode'` argument, it calculates one of the following Ripley's statistics: - `{rp.F.s!r}`, `{rp.G.s!r}` or `{rp.L.s!r}` statistics. + `'F'`, `'G'` or `'L'` statistics. - `{rp.F.s!r}`, `{rp.G.s!r}` are defined as: + `'F'`, `'G'` are defined as: .. math:: @@ -73,10 +73,10 @@ def ripley( Where :math:`d_{{i,j}}` represents: - - distances to a random Spatial Poisson Point Process for `{rp.F.s!r}`. - - distances to any other point of the dataset for `{rp.G.s!r}`. + - distances to a random Spatial Poisson Point Process for `'F'`. + - distances to any other point of the dataset for `'G'`. - `{rp.L.s!r}` we first need to compute :math:`K(t)`, which is defined as: + `'L'` we first need to compute :math:`K(t)`, which is defined as: .. math:: @@ -132,9 +132,7 @@ def ripley( coordinates = adata.obsm[spatial_key] clusters = adata.obs[cluster_key].values - mode = RipleyStat(mode) # type: ignore[assignment] - if TYPE_CHECKING: - assert isinstance(mode, RipleyStat) + mode = normalize_choice(mode, ("F", "G", "L"), name="mode") # prepare support N = coordinates.shape[0] @@ -156,19 +154,19 @@ def ripley( for i in np.arange(np.max(cluster_idx) + 1): coord_c = coordinates[cluster_idx == i, :] - if mode == RipleyStat.F: + if mode == "F": random = _ppp(hull, n_simulations=1, n_observations=n_observations, rng=obs_rng) tree_c = NearestNeighbors(metric=metric, n_neighbors=n_neigh).fit(coord_c) distances, _ = tree_c.kneighbors(random, n_neighbors=n_neigh) bins, obs_stats = _f_g_function(distances.squeeze(), support) - elif mode == RipleyStat.G: + elif mode == "G": tree_c = NearestNeighbors(metric=metric, n_neighbors=n_neigh).fit(coord_c) distances, _ = tree_c.kneighbors(coordinates[cluster_idx != i, :], n_neighbors=n_neigh) bins, obs_stats = _f_g_function(distances.squeeze(), support) - elif mode == RipleyStat.L: + elif mode == "L": bins, obs_stats = _l_function(coord_c, support, N, area, metric) else: - raise NotImplementedError(f"Mode `{mode.s!r}` is not yet implemented.") + raise NotImplementedError(f"Mode `{mode!r}` is not yet implemented.") obs_arr[i] = obs_stats sims = np.empty((n_simulations, len(bins))) @@ -176,18 +174,18 @@ def ripley( for i in range(n_simulations): random_i = _ppp(hull, n_simulations=1, n_observations=n_observations, rng=sim_rngs[i]) - if mode == RipleyStat.F: + if mode == "F": tree_i = NearestNeighbors(metric=metric, n_neighbors=n_neigh).fit(random_i) distances_i, _ = tree_i.kneighbors(random, n_neighbors=1) _, stats_i = _f_g_function(distances_i.squeeze(), support) - elif mode == RipleyStat.G: + elif mode == "G": tree_i = NearestNeighbors(metric=metric, n_neighbors=n_neigh).fit(random_i) distances_i, _ = tree_i.kneighbors(coordinates, n_neighbors=1) _, stats_i = _f_g_function(distances_i.squeeze(), support) - elif mode == RipleyStat.L: + elif mode == "L": _, stats_i = _l_function(random_i, support, N, area, metric) else: - raise NotImplementedError(f"Mode `{mode.s!r}` is not yet implemented.") + raise NotImplementedError(f"Mode `{mode!r}` is not yet implemented.") for j in range(obs_arr.shape[0]): pvalues[j] += stats_i >= obs_arr[j] diff --git a/src/squidpy/gr/neighbors.py b/src/squidpy/gr/neighbors.py index 72e848624..971f619b3 100644 --- a/src/squidpy/gr/neighbors.py +++ b/src/squidpy/gr/neighbors.py @@ -9,7 +9,7 @@ from abc import ABC, abstractmethod from collections.abc import Callable, Sequence from dataclasses import dataclass -from typing import Any, TypeVar, cast +from typing import Any, Literal, TypeVar, cast import numpy as np from fast_array_utils import stats as fau_stats @@ -26,9 +26,9 @@ from sklearn.metrics.pairwise import cosine_similarity from sklearn.neighbors import NearestNeighbors -from squidpy._constants._constants import CoordType, Transform +from squidpy._constants._constants import Transform from squidpy._utils import NDArrayA -from squidpy._validators import assert_positive +from squidpy._validators import assert_positive, normalize_choice from squidpy.gr._nhood import compute_hop_adjacency_matrices __all__ = [ @@ -45,6 +45,14 @@ "GridBuilder", ] +TransformLike = Literal["spectral", "cosine"] | None +"""Adjacency-matrix transform, or `None` for no transform. Case-insensitive.""" + + +def _as_transform(transform: TransformLike | Transform) -> Transform: + """Resolve the ``transform`` argument to its enum member, accepting any casing.""" + return Transform(normalize_choice(transform, [m.value for m in Transform], name="transform")) + # Kept module-level (not folded into GraphBuilder's params): types the public # `GraphPostprocessor` alias and is itself a public `squidpy.gr` export. @@ -63,12 +71,12 @@ class GraphBuilder[CoordT, GraphMatrixT](ABC): def __init__( self, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, percentile: float | None = None, postprocessors: Sequence[GraphPostprocessor[GraphMatrixT]] = (), ) -> None: - self.transform = Transform.NONE if transform is None else Transform(transform) + self.transform = _as_transform(transform) self.set_diag = set_diag self.percentile = percentile self._postprocessors: list[GraphPostprocessor[GraphMatrixT]] = list(postprocessors) @@ -166,7 +174,7 @@ class KNNBuilder(GraphBuilderCSR): def __init__( self, n_neighs: int = 6, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, percentile: float | None = None, ) -> None: @@ -174,7 +182,7 @@ def __init__( postprocessors: list[GraphPostprocessor[csr_matrix]] = [] if percentile is not None: postprocessors.append(PercentilePostprocessor(percentile)) - postprocessors.append(TransformPostprocessor(Transform.NONE if transform is None else Transform(transform))) + postprocessors.append(TransformPostprocessor(_as_transform(transform))) super().__init__( transform=transform, set_diag=set_diag, @@ -185,7 +193,7 @@ def __init__( def uns_params(self) -> dict[str, Any]: return { - "coord_type": CoordType.GENERIC.v, + "coord_type": "generic", "n_neighbors": self.n_neighs, "transform": self.transform.v, } @@ -222,7 +230,7 @@ class RadiusBuilder(GraphBuilderCSR): def __init__( self, radius: float | tuple[float, float], - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, percentile: float | None = None, ) -> None: @@ -231,7 +239,7 @@ def __init__( postprocessors.append(DistanceIntervalPostprocessor(tuple(sorted(radius)))) if percentile is not None: postprocessors.append(PercentilePostprocessor(percentile)) - postprocessors.append(TransformPostprocessor(Transform.NONE if transform is None else Transform(transform))) + postprocessors.append(TransformPostprocessor(_as_transform(transform))) super().__init__( transform=transform, set_diag=set_diag, @@ -243,7 +251,7 @@ def __init__( def uns_params(self) -> dict[str, Any]: return { - "coord_type": CoordType.GENERIC.v, + "coord_type": "generic", "radius": self.radius, "transform": self.transform.v, } @@ -290,7 +298,7 @@ class DelaunayBuilder(GraphBuilderCSR): def __init__( self, radius: float | tuple[float, float] | None = None, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, percentile: float | None = None, ) -> None: @@ -301,7 +309,7 @@ def __init__( postprocessors.append(DistanceIntervalPostprocessor(tuple(sorted(radius)))) if percentile is not None: postprocessors.append(PercentilePostprocessor(percentile)) - postprocessors.append(TransformPostprocessor(Transform.NONE if transform is None else Transform(transform))) + postprocessors.append(TransformPostprocessor(_as_transform(transform))) super().__init__( transform=transform, set_diag=set_diag, @@ -313,7 +321,7 @@ def __init__( def uns_params(self) -> dict[str, Any]: return { - "coord_type": CoordType.GENERIC.v, + "coord_type": "generic", "radius": self.radius, "transform": self.transform.v, } @@ -350,12 +358,12 @@ def __init__( n_neighs: int = 6, n_rings: int = 1, delaunay: bool = False, - transform: str | Transform | None = None, + transform: TransformLike = None, set_diag: bool = False, ) -> None: assert_positive(n_neighs, name="n_neighs") assert_positive(n_rings, name="n_rings") - postprocessors = [TransformPostprocessor(Transform.NONE if transform is None else Transform(transform))] + postprocessors = [TransformPostprocessor(_as_transform(transform))] super().__init__(transform=transform, set_diag=set_diag, percentile=None, postprocessors=postprocessors) self.n_neighs = n_neighs self.n_rings = n_rings @@ -363,7 +371,7 @@ def __init__( def uns_params(self) -> dict[str, Any]: return { - "coord_type": CoordType.GRID.v, + "coord_type": "grid", "n_neighbors": self.n_neighs, "n_rings": self.n_rings, "delaunay": self.delaunay, diff --git a/src/squidpy/pl/_graph.py b/src/squidpy/pl/_graph.py index dcca7d8bb..13f3a2f44 100644 --- a/src/squidpy/pl/_graph.py +++ b/src/squidpy/pl/_graph.py @@ -6,7 +6,7 @@ from collections.abc import Mapping, Sequence from pathlib import Path from types import MappingProxyType -from typing import TYPE_CHECKING, Any, Literal +from typing import Any, Literal import matplotlib.pyplot as plt import numpy as np @@ -17,7 +17,6 @@ from matplotlib.axes import Axes from squidpy._compat import old_positionals -from squidpy._constants._constants import RipleyStat from squidpy._constants._pkg_constants import Key from squidpy._docs import d from squidpy._validators import assert_non_empty_sequence, get_valid_values, normalize_choice @@ -431,10 +430,8 @@ def ripley( """ _assert_categorical_obs(adata, key=cluster_key) - mode = RipleyStat(mode) # type: ignore[assignment] - if TYPE_CHECKING: - assert isinstance(mode, RipleyStat) - res = _get_data(adata, cluster_key=cluster_key, func_name="ripley", mode=mode.s) + mode = normalize_choice(mode, ("F", "G", "L"), name="mode") + res = _get_data(adata, cluster_key=cluster_key, func_name="ripley", mode=mode) legend_kwargs = dict(legend_kwargs) if "loc" not in legend_kwargs: @@ -451,7 +448,7 @@ def ripley( y="stats", x="bins", hue=cluster_key, - data=res[f"{mode.s}_stat"], + data=res[f"{mode}_stat"], hue_order=categories, palette=palette, ax=ax, @@ -461,7 +458,7 @@ def ripley( sns.lineplot(y="stats", x="bins", errorbar="sd", alpha=0.01, color="gray", data=res["sims_stat"], ax=ax) ax.legend(**legend_kwargs) ax.set_ylabel("value") - ax.set_title(f"Ripley's {mode.s}") + ax.set_title(f"Ripley's {mode}") if save is not None: save_fig(fig, path=save) diff --git a/tests/graph/test_ripley.py b/tests/graph/test_ripley.py index 8daf909ed..4d0938745 100644 --- a/tests/graph/test_ripley.py +++ b/tests/graph/test_ripley.py @@ -4,7 +4,6 @@ import pytest from anndata import AnnData -from squidpy._constants._constants import RipleyStat from squidpy.gr import ripley CLUSTER_KEY = "leiden" @@ -15,11 +14,11 @@ def test_ripley_L_unsupported_metric_raises(adata_ripley: AnnData): ripley(adata_ripley, cluster_key=CLUSTER_KEY, mode="L", metric="cosine") -@pytest.mark.parametrize("mode", list(RipleyStat)) -def test_ripley_modes(adata_ripley: AnnData, mode: RipleyStat): +@pytest.mark.parametrize("mode", ["F", "G", "L"]) +def test_ripley_modes(adata_ripley: AnnData, mode: str): adata = adata_ripley - ripley(adata, cluster_key=CLUSTER_KEY, mode=mode.s) + ripley(adata, cluster_key=CLUSTER_KEY, mode=mode) UNS_KEY = f"{CLUSTER_KEY}_ripley_{mode}" @@ -42,7 +41,7 @@ def test_ripley_modes(adata_ripley: AnnData, mode: RipleyStat): assert bins.shape[0] == 50 -@pytest.mark.parametrize("mode", list(RipleyStat)) +@pytest.mark.parametrize("mode", ["F", "G", "L"]) @pytest.mark.parametrize( "n_simulations", [20, 50], @@ -60,7 +59,7 @@ def test_ripley_modes(adata_ripley: AnnData, mode: RipleyStat): [2, 50, 100], ) def test_ripley_results( - adata_ripley: AnnData, mode: RipleyStat, n_simulations: int, n_observations: int, max_dist: np.float_, n_steps: int + adata_ripley: AnnData, mode: str, n_simulations: int, n_observations: int, max_dist: np.float_, n_steps: int ): adata = adata_ripley n_clusters = adata.obs[CLUSTER_KEY].cat.categories.shape[0] @@ -68,7 +67,7 @@ def test_ripley_results( res = ripley( adata, cluster_key=CLUSTER_KEY, - mode=mode.s, + mode=mode, n_simulations=n_simulations, n_observations=n_observations, max_dist=max_dist, @@ -98,11 +97,11 @@ def test_ripley_results( assert idx.shape[0] == n_steps * n_clusters - n_clusters -@pytest.mark.parametrize("mode", [RipleyStat.F, RipleyStat.G, RipleyStat.L]) -def test_ripley_rng(adata_ripley: AnnData, mode: RipleyStat): +@pytest.mark.parametrize("mode", ["F", "G", "L"]) +def test_ripley_rng(adata_ripley: AnnData, mode: str): """Same seed reproduces simulations, different seeds change them, and simulations are not all identical.""" adata = adata_ripley - kw = {"cluster_key": CLUSTER_KEY, "mode": mode.s, "n_simulations": 20, "copy": True} + kw = {"cluster_key": CLUSTER_KEY, "mode": mode, "n_simulations": 20, "copy": True} res1 = ripley(adata, rng=np.random.default_rng(42), **kw) res2 = ripley(adata, rng=np.random.default_rng(42), **kw) diff --git a/tests/test_string_choices.py b/tests/test_string_choices.py index 3ff9a02be..fed94e817 100644 --- a/tests/test_string_choices.py +++ b/tests/test_string_choices.py @@ -67,25 +67,25 @@ def _pl_ripley(adata, mode): "nhood_data", lambda a, v: sq.gr.spatial_autocorr(a, mode=v, genes=a.var_names[:2], copy=True), "GEARY", - "`SpatialAutocorr`", + "`mode`", ), "gr.ripley-mode": ( "nhood_data", lambda a, v: sq.gr.ripley(a, cluster_key="leiden", mode=v, n_simulations=2, n_steps=5, rng=0, copy=True), "l", - "`RipleyStat`", + "`mode`", ), "gr.spatial_neighbors-coord_type": ( "nhood_data", lambda a, v: sq.gr.spatial_neighbors(a, coord_type=v, copy=True), "GENERIC", - "`CoordType`", + "`coord_type`", ), "gr.spatial_neighbors_knn-transform": ( "nhood_data", lambda a, v: sq.gr.spatial_neighbors_knn(a, transform=v, copy=True), "COSINE", - "`Transform`", + "`transform`", ), "gr.calculate_niche_cellcharter-aggregation": ( "dummy_adata2", @@ -100,7 +100,7 @@ def _pl_ripley(adata, mode): "`flavor`", ), "pl.nhood_enrichment-mode": ("nhood_data", _pl_nhood, "COUNT", "`mode`"), - "pl.ripley-mode": ("nhood_data", _pl_ripley, "g", "`RipleyStat`"), + "pl.ripley-mode": ("nhood_data", _pl_ripley, "g", "`mode`"), "experimental.pl.qc_image-metrics": (None, _pl_qc_image, "TENENGRAD", "`metrics`"), "datasets.visium-sample_id": ( "monkeypatch", @@ -171,3 +171,10 @@ def test_pl_centrality_scores_score_is_case_insensitive(nhood_data) -> None: sq.pl.centrality_scores(nhood_data, cluster_key="leiden", score="DEGREE_CENTRALITY") with pytest.raises(ValueError, match="No valid values"): sq.pl.centrality_scores(nhood_data, cluster_key="leiden", score="bogus") + + +def test_enum_members_are_still_accepted(nhood_data) -> None: + """``Transform`` is private but was named in released signatures, so members keep working.""" + from squidpy._constants._constants import Transform + + sq.gr.spatial_neighbors_knn(nhood_data, transform=Transform.COSINE, copy=True) From f9446ee57274bc29cf7798fe756eedfec944fbd5 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 19:53:46 +0200 Subject: [PATCH 07/26] Trim dead enums and redundant docs Delete four ModeEnum subclasses with no remaining references, drop the transform normalization _build.py repeated on top of the builders' own, and remove branches normalize_choice already made unreachable. gr.ripley loses an @inject_docs that injected nothing, so its LaTeX no longer needs doubled braces. Option lists that only restate a Literal go; Sphinx renders the annotation into the signature already. pl.centrality_scores folds the requested score names against the columns directly instead of catching ValueError from normalize_choice. Note in contributing.md which arguments are still enum-backed, and so still case-sensitive, now that ModeEnum._missing_ is gone. --- docs/contributing.md | 4 ++++ src/squidpy/_constants/_constants.py | 27 -------------------------- src/squidpy/gr/_build.py | 29 +++++++--------------------- src/squidpy/gr/_ppatterns.py | 4 +--- src/squidpy/gr/_ripley.py | 26 +++++++++---------------- src/squidpy/pl/_graph.py | 17 ++++------------ 6 files changed, 25 insertions(+), 82 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 9c8bd62f4..f4974fd56 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -172,6 +172,10 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors - **Keyword-only arguments**: a public function takes its data object (`adata`, `sdata` or `data`) positionally, and every other argument is keyword-only, e.g. `def nhood_enrichment(adata, *, cluster_key, ...)`. Avoid catch-all `**kwargs` that forward whatever they are given unchecked. - **Parameter bags**: a `*Params` {class}`~typing.TypedDict` in {mod}`squidpy.types`, each key declaring its default as `Annotated[type, Default(value)]`, only when the knobs depend on another argument (one bag per `method`, taken as `method_params`) or several functions share them (taken as `**kwargs: Unpack[XParams]`). Resolve it with `resolve_params` from `squidpy._params`, so unknown keys raise, and keep its keys out of the signature. - **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. + + Two exceptions. The first is temporary: arguments still backed by a `ModeEnum` from `squidpy._constants._constants` are case-**sensitive**, because they are handed to `SomeEnum(value)` with no `normalize_choice` in front. As of this writing that is `score` ({func}`~squidpy.gr.centrality_scores`), `shape` ({func}`~squidpy.pl.spatial_scatter`), `corr_axis` and `complex_policy` ({func}`~squidpy.gr.ligrec`), `dendrogram` ({func}`~squidpy.pl.ligrec`), `features` ({func}`~squidpy.im.calculate_image_features`), `method` ({func}`~squidpy.im.process` and {func}`~squidpy.im.segment`), and `dims` ({class}`~squidpy.im.ImageContainer`). Converting the argument to a `Literal` resolved by `normalize_choice` is what fixes it — do that rather than reintroducing a case-folding hook on the enum base class, which would hide how many arguments are still unconverted. + + The second is permanent: a name matched against *runtime* data rather than a fixed option list cannot use `normalize_choice`, since the options are not known until the call. Fold such a name against the data at the call site instead — {func}`~squidpy.pl.centrality_scores` matches `score` case-insensitively against the computed columns that way. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index f5c40d141..78f082537 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -51,19 +51,6 @@ class SegmentationBackend(ModeEnum): CUSTOM = "custom" # callable function -@unique -class BlobModel(ModeEnum): - LOG = "log" - DOG = "dog" - DOH = "doh" - - -@unique -class Dataset(ModeEnum): - OB = "ob" - SVZ = "svz" - - @unique class Centrality(ModeEnum): DEGREE = "degree_centrality" @@ -78,12 +65,6 @@ class DendrogramAxis(ModeEnum): BOTH = "both" -@unique -class Symbol(ModeEnum): - DISC = "disc" - SQUARE = "square" - - @unique class InferDimensions(ModeEnum): DEFAULT = "default" @@ -98,14 +79,6 @@ class ScatterShape(str, ModeEnum): HEX = "hex" -@unique -class TenxVersions(str, ModeEnum): - # Version numbers as class objects of TenxVersions - V1 = "1.1.0" - V2 = "1.2.0" - V3 = "1.3.0" - - @unique class NicheDefinitions(ModeEnum): NEIGHBORHOOD = "neighborhood" diff --git a/src/squidpy/gr/_build.py b/src/squidpy/gr/_build.py index 1f6e5a7db..a33a256d7 100644 --- a/src/squidpy/gr/_build.py +++ b/src/squidpy/gr/_build.py @@ -42,7 +42,6 @@ KNNBuilder, RadiusBuilder, TransformLike, - _as_transform, ) __all__ = [ @@ -84,7 +83,6 @@ def _resolve_graph_builder( assert_positive(n_rings, name="n_rings") assert_positive(n_neighs, name="n_neighs") - transform = _as_transform(transform) if coord_type is None: if radius is not None: logg.warning( @@ -195,12 +193,8 @@ def spatial_neighbors( `adata` is a :class:`spatialdata.SpatialData`. %(library_key)s coord_type - Type of coordinate system. Valid options are: - - - `'grid'` - grid coordinates. - - `'generic'` - generic coordinates. - - `None` - `'grid'` if ``spatial_key`` is in :attr:`anndata.AnnData.uns` - with ``n_neighs = 6`` (Visium), otherwise use `'generic'`. + Type of coordinate system. If `None`, use `'grid'` when ``spatial_key`` is in + :attr:`anndata.AnnData.uns` with ``n_neighs = 6`` (Visium), otherwise `'generic'`. n_neighs Depending on the ``coord_type``: @@ -223,12 +217,7 @@ def spatial_neighbors( percentile Percentile of the distances to use as threshold. Only used when ``coord_type = 'generic'``. transform - Type of adjacency matrix transform. - Valid options are: - - - `'spectral'` - spectral transformation of the adjacency matrix. - - `'cosine'` - cosine transformation of the adjacency matrix. - - `None` - no transformation of the adjacency matrix. + Adjacency matrix transform, or `None` for no transform. set_diag Whether to set the diagonal of the spatial connectivities to `1.0`. key_added @@ -555,11 +544,10 @@ def spatial_neighbors_knn( spatial_neighbors_from_builder : Use :class:`~squidpy.gr.neighbors.KNNBuilder` directly for advanced customization. squidpy.gr.neighbors.KNNBuilder : k-nearest-neighbor builder class. """ - transform_enum = _as_transform(transform) builder = KNNBuilder( n_neighs=n_neighs, percentile=percentile, - transform=transform_enum, + transform=transform, set_diag=set_diag, ) adata, library_key = _prepare_spatial_neighbors_input( @@ -628,11 +616,10 @@ def spatial_neighbors_radius( spatial_neighbors_from_builder : Use :class:`~squidpy.gr.neighbors.RadiusBuilder` directly for advanced customization. squidpy.gr.neighbors.RadiusBuilder : radius-based builder class. """ - transform_enum = _as_transform(transform) builder = RadiusBuilder( radius=radius, percentile=percentile, - transform=transform_enum, + transform=transform, set_diag=set_diag, ) adata, library_key = _prepare_spatial_neighbors_input( @@ -705,11 +692,10 @@ def spatial_neighbors_delaunay( spatial_neighbors_from_builder : Use :class:`~squidpy.gr.neighbors.DelaunayBuilder` directly for advanced customization. squidpy.gr.neighbors.DelaunayBuilder : Delaunay triangulation builder class. """ - transform_enum = _as_transform(transform) builder = DelaunayBuilder( radius=radius, percentile=percentile, - transform=transform_enum, + transform=transform, set_diag=set_diag, ) adata, library_key = _prepare_spatial_neighbors_input( @@ -794,12 +780,11 @@ def spatial_neighbors_grid( """ assert_positive(n_rings, name="n_rings") assert_positive(n_neighs, name="n_neighs") - transform_enum = _as_transform(transform) builder = GridBuilder( n_neighs=n_neighs, n_rings=n_rings, delaunay=delaunay, - transform=transform_enum, + transform=transform, set_diag=set_diag, ) adata, library_key = _prepare_spatial_neighbors_input( diff --git a/src/squidpy/gr/_ppatterns.py b/src/squidpy/gr/_ppatterns.py index 922ad3739..152a7e9d3 100644 --- a/src/squidpy/gr/_ppatterns.py +++ b/src/squidpy/gr/_ppatterns.py @@ -211,10 +211,8 @@ def extract_obsm(adata: AnnData, ixs: int | Sequence[int] | None) -> tuple[NDArr vals, index = extract_X(adata, genes) # type: ignore elif attr == "obs": vals, index = extract_obs(adata, genes) # type: ignore - elif attr == "obsm": - vals, index = extract_obsm(adata, genes) # type: ignore else: - raise NotImplementedError(f"Extracting from `adata.{attr}` is not yet implemented.") + vals, index = extract_obsm(adata, genes) # type: ignore mode = normalize_choice(mode, ("moran", "geary"), name="mode") params = {"mode": mode, "transformation": transformation, "two_tailed": two_tailed} diff --git a/src/squidpy/gr/_ripley.py b/src/squidpy/gr/_ripley.py index 0048310ab..0b13cf038 100644 --- a/src/squidpy/gr/_ripley.py +++ b/src/squidpy/gr/_ripley.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Literal +from typing import Literal import numpy as np import pandas as pd @@ -15,7 +15,7 @@ from squidpy._compat import old_positionals from squidpy._constants._pkg_constants import Key -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import NDArrayA, RNGLike, SeedLike, deprecated_randomness_param from squidpy._validators import normalize_choice from squidpy.gr._utils import _assert_categorical_obs, _assert_spatial_basis, _save_data, extract_adata_if_sdata @@ -24,7 +24,6 @@ @d.dedent -@inject_docs(key=Key.obsm.spatial) @old_positionals( "cluster_key", "mode", @@ -69,9 +68,9 @@ def ripley( .. math:: - F(t),G(t)=P( d_{{i,j}} \le t ) + F(t),G(t)=P( d_{i,j} \le t ) - Where :math:`d_{{i,j}}` represents: + Where :math:`d_{i,j}` represents: - distances to a random Spatial Poisson Point Process for `'F'`. - distances to any other point of the dataset for `'G'`. @@ -80,14 +79,14 @@ def ripley( .. math:: - K(t) = \frac{{1}}{{\lambda}} \sum_{{i \ne j}} \frac{{I(d_{{i,j}}= obs_arr[j] @@ -199,9 +194,6 @@ def ripley( res = {f"{mode}_stat": obs_df, "sims_stat": sims_df, "bins": bins, "pvalues": pvalues} - if TYPE_CHECKING: - assert isinstance(res, dict) - if copy: logg.info("Finish", time=start) return res diff --git a/src/squidpy/pl/_graph.py b/src/squidpy/pl/_graph.py index 13f3a2f44..8f6b46c7b 100644 --- a/src/squidpy/pl/_graph.py +++ b/src/squidpy/pl/_graph.py @@ -101,14 +101,9 @@ def centrality_scores( score = scores if score is None else score score = assert_non_empty_sequence(score, name="centrality scores") - def _canonical(s: str) -> str: - # Case-insensitive match; unknown names pass through and are dropped by get_valid_values. - try: - return normalize_choice(s, scores, name="score") - except ValueError: - return s - - score = sorted(get_valid_values([_canonical(s) for s in score], scores)) + # Match the column names case-insensitively; unknown names stay as-is and get dropped below. + by_fold = {str(s).casefold(): s for s in scores} + score = sorted(get_valid_values([by_fold.get(str(s).casefold(), s) for s in score], scores)) fig, axs = plt.subplots(1, len(score), figsize=figsize, dpi=dpi, constrained_layout=True) axs = np.ravel(axs) # make into iterable @@ -237,11 +232,7 @@ def nhood_enrichment( %(adata)s %(cluster_key)s mode - Which :func:`squidpy.gr.nhood_enrichment` result to plot. Valid options are: - - - `'zscore'` - z-score values of enrichment statistic. - - `'count'` - enrichment count. - + Which :func:`squidpy.gr.nhood_enrichment` result to plot. %(heatmap_plotting)s kwargs Keyword arguments for :func:`matplotlib.pyplot.text`. From 95c47d721552c1a0e5aa5ecb966f298b1d1d6a37 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:00:25 +0200 Subject: [PATCH 08/26] Drop the Transform enum and the Enum shim TransformPostprocessor holds the canonical string, so the last ModeEnum behind a converted argument goes, along with the Enum unwrap in normalize_choice that existed only to keep Transform members working from a private module. The uns payload is unchanged: .v already yielded these same strings and None. --- docs/conf.py | 2 -- src/squidpy/_constants/_constants.py | 7 ------- src/squidpy/_validators.py | 3 --- src/squidpy/gr/neighbors.py | 23 +++++++++++------------ tests/graph/test_spatial_neighbors.py | 3 +-- tests/test_string_choices.py | 7 ------- 6 files changed, 12 insertions(+), 33 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index a42908d7c..bdb4a8dc3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -146,8 +146,6 @@ ("py:class", "numpy._typing._array_like.NDArray"), ("py:class", "np.number"), ("py:class", "csr_matrix"), - # no idea why those aren’t exported - ("py:class", "squidpy._constants._constants.Transform"), ("py:class", "pandas.core.frame.DataFrame"), ] # see the solution from: https://github.com/sphinx-doc/sphinx/issues/7369 diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index 78f082537..6278cbcc5 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -29,13 +29,6 @@ class ComplexPolicy(ModeEnum): ALL = "all" -@unique -class Transform(ModeEnum): - SPECTRAL = "spectral" - COSINE = "cosine" - NONE = None - - @unique class Processing(ModeEnum): SMOOTH = "smooth" diff --git a/src/squidpy/_validators.py b/src/squidpy/_validators.py index af39afef7..b84b59ede 100644 --- a/src/squidpy/_validators.py +++ b/src/squidpy/_validators.py @@ -3,7 +3,6 @@ from __future__ import annotations from collections.abc import Hashable, Iterable, Sequence -from enum import Enum from typing import TYPE_CHECKING, Any from squidpy._utils import _unique_order_preserving @@ -97,8 +96,6 @@ def normalize_choice(value: Any, options: Iterable[Any], *, name: str) -> Any: Raise ValueError naming the valid options if nothing matches. """ options = list(options) - if isinstance(value, Enum): - value = value.value if isinstance(value, str): for opt in options: if isinstance(opt, str) and opt.casefold() == value.casefold(): diff --git a/src/squidpy/gr/neighbors.py b/src/squidpy/gr/neighbors.py index 971f619b3..ab0932acf 100644 --- a/src/squidpy/gr/neighbors.py +++ b/src/squidpy/gr/neighbors.py @@ -26,7 +26,6 @@ from sklearn.metrics.pairwise import cosine_similarity from sklearn.neighbors import NearestNeighbors -from squidpy._constants._constants import Transform from squidpy._utils import NDArrayA from squidpy._validators import assert_positive, normalize_choice from squidpy.gr._nhood import compute_hop_adjacency_matrices @@ -49,9 +48,9 @@ """Adjacency-matrix transform, or `None` for no transform. Case-insensitive.""" -def _as_transform(transform: TransformLike | Transform) -> Transform: - """Resolve the ``transform`` argument to its enum member, accepting any casing.""" - return Transform(normalize_choice(transform, [m.value for m in Transform], name="transform")) +def _as_transform(transform: TransformLike) -> TransformLike: + """Resolve the ``transform`` argument to its canonical spelling, accepting any casing.""" + return normalize_choice(transform, ("spectral", "cosine", None), name="transform") # Kept module-level (not folded into GraphBuilder's params): types the public @@ -195,7 +194,7 @@ def uns_params(self) -> dict[str, Any]: return { "coord_type": "generic", "n_neighbors": self.n_neighs, - "transform": self.transform.v, + "transform": self.transform, } def build_graph(self, coords: NDArrayA) -> tuple[csr_matrix, csr_matrix]: @@ -253,7 +252,7 @@ def uns_params(self) -> dict[str, Any]: return { "coord_type": "generic", "radius": self.radius, - "transform": self.transform.v, + "transform": self.transform, } def build_graph(self, coords: NDArrayA) -> tuple[csr_matrix, csr_matrix]: @@ -323,7 +322,7 @@ def uns_params(self) -> dict[str, Any]: return { "coord_type": "generic", "radius": self.radius, - "transform": self.transform.v, + "transform": self.transform, } def build_graph(self, coords: NDArrayA) -> tuple[csr_matrix, csr_matrix]: @@ -375,7 +374,7 @@ def uns_params(self) -> dict[str, Any]: "n_neighbors": self.n_neighs, "n_rings": self.n_rings, "delaunay": self.delaunay, - "transform": self.transform.v, + "transform": self.transform, } def build_graph(self, coords: NDArrayA) -> tuple[csr_matrix, csr_matrix]: @@ -468,17 +467,17 @@ def __call__(self, adj: csr_matrix, dst: csr_matrix) -> tuple[csr_matrix, csr_ma @dataclass(frozen=True) class TransformPostprocessor: - transform: Transform + transform: TransformLike def __call__(self, adj: csr_matrix, dst: csr_matrix) -> tuple[csr_matrix, csr_matrix]: adj.eliminate_zeros() dst.eliminate_zeros() - if self.transform == Transform.SPECTRAL: + if self.transform == "spectral": return cast(csr_matrix, _transform_a_spectral(adj)), dst - if self.transform == Transform.COSINE: + if self.transform == "cosine": return cast(csr_matrix, _transform_a_cosine(adj)), dst - if self.transform == Transform.NONE: + if self.transform is None: return adj, dst raise NotImplementedError(f"Transform `{self.transform}` is not yet implemented.") diff --git a/tests/graph/test_spatial_neighbors.py b/tests/graph/test_spatial_neighbors.py index a92a8c03f..81babc36a 100644 --- a/tests/graph/test_spatial_neighbors.py +++ b/tests/graph/test_spatial_neighbors.py @@ -10,7 +10,6 @@ from shapely import Point from spatialdata.datasets import blobs -from squidpy._constants._constants import Transform from squidpy._constants._pkg_constants import Key from squidpy.gr import ( mask_graph, @@ -246,7 +245,7 @@ def test_grid_builder_matches_legacy(self, adata_squaregrid: AnnData, legacy_kwa np.testing.assert_allclose(legacy.distances.toarray(), result.distances.toarray()) def test_builder_explicit_entry_point(self, non_visium_adata: AnnData): - builder = KNNBuilder(n_neighs=3, transform=Transform.NONE) + builder = KNNBuilder(n_neighs=3, transform=None) baseline = spatial_neighbors_from_builder(non_visium_adata, builder=builder, copy=True) matched = spatial_neighbors_from_builder(non_visium_adata, builder=builder, copy=True) diff --git a/tests/test_string_choices.py b/tests/test_string_choices.py index fed94e817..944776d66 100644 --- a/tests/test_string_choices.py +++ b/tests/test_string_choices.py @@ -171,10 +171,3 @@ def test_pl_centrality_scores_score_is_case_insensitive(nhood_data) -> None: sq.pl.centrality_scores(nhood_data, cluster_key="leiden", score="DEGREE_CENTRALITY") with pytest.raises(ValueError, match="No valid values"): sq.pl.centrality_scores(nhood_data, cluster_key="leiden", score="bogus") - - -def test_enum_members_are_still_accepted(nhood_data) -> None: - """``Transform`` is private but was named in released signatures, so members keep working.""" - from squidpy._constants._constants import Transform - - sq.gr.spatial_neighbors_knn(nhood_data, transform=Transform.COSINE, copy=True) From 384d3b901a8f0df35249fbbcd352a9069af9f36a Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:10:51 +0200 Subject: [PATCH 09/26] Let ModeEnum members be strings Transform.NONE was the only non-string value, so the str mixin the TODO asked for now applies cleanly: members compare and hash equal to their value, and ScatterShape drops its own str prefix. Narrow the TODO to what is left, the 63 .s call sites. Also say plainly why pl.centrality_scores cannot use normalize_choice: its valid values are whichever columns gr.centrality_scores wrote. --- docs/contributing.md | 2 +- src/squidpy/_constants/_constants.py | 2 +- src/squidpy/_constants/_utils.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index f4974fd56..70d562b0a 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -175,7 +175,7 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors Two exceptions. The first is temporary: arguments still backed by a `ModeEnum` from `squidpy._constants._constants` are case-**sensitive**, because they are handed to `SomeEnum(value)` with no `normalize_choice` in front. As of this writing that is `score` ({func}`~squidpy.gr.centrality_scores`), `shape` ({func}`~squidpy.pl.spatial_scatter`), `corr_axis` and `complex_policy` ({func}`~squidpy.gr.ligrec`), `dendrogram` ({func}`~squidpy.pl.ligrec`), `features` ({func}`~squidpy.im.calculate_image_features`), `method` ({func}`~squidpy.im.process` and {func}`~squidpy.im.segment`), and `dims` ({class}`~squidpy.im.ImageContainer`). Converting the argument to a `Literal` resolved by `normalize_choice` is what fixes it — do that rather than reintroducing a case-folding hook on the enum base class, which would hide how many arguments are still unconverted. - The second is permanent: a name matched against *runtime* data rather than a fixed option list cannot use `normalize_choice`, since the options are not known until the call. Fold such a name against the data at the call site instead — {func}`~squidpy.pl.centrality_scores` matches `score` case-insensitively against the computed columns that way. + The second is permanent: `normalize_choice` needs an option list, and some arguments do not have one until the call runs. {func}`~squidpy.pl.centrality_scores` accepts whichever columns {func}`~squidpy.gr.centrality_scores` happened to write into {attr}`~anndata.AnnData.uns`, so its valid values come from the data, not the signature. Case-fold the user's value against the data where you read it — `{c.casefold(): c for c in columns}` — rather than reaching for `normalize_choice`. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index 6278cbcc5..436301acb 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -66,7 +66,7 @@ class InferDimensions(ModeEnum): @unique -class ScatterShape(str, ModeEnum): +class ScatterShape(ModeEnum): CIRCLE = "circle" SQUARE = "square" HEX = "hex" diff --git a/src/squidpy/_constants/_utils.py b/src/squidpy/_constants/_utils.py index 7c0fa2feb..865facbc4 100644 --- a/src/squidpy/_constants/_utils.py +++ b/src/squidpy/_constants/_utils.py @@ -61,8 +61,8 @@ def __new__( # noqa: D102 return res -# TODO(michalk8): subclass string; remove .s? -class ModeEnum(ErrorFormatterABC, PrettyEnum, metaclass=ABCEnumMeta): +# TODO(michalk8): members now compare equal to their value; `.s` is redundant at its 63 call sites. +class ModeEnum(str, ErrorFormatterABC, PrettyEnum, metaclass=ABCEnumMeta): """Enum which prints available values when invalid value has been passed.""" @property From c93bd5668ac680684cbf85b862985261bb66da74 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:23:15 +0200 Subject: [PATCH 10/26] Convert centrality score to a Literal The score columns are the three Centrality values, a closed set, so the argument takes a Literal on both the gr and pl side and resolves through normalize_choice. That retires the enum, the unreachable NotImplementedError guard over its own three members, and four of the seven real .s call sites. Correct the contributing note that claimed this one could not have an option list, and cover gr.centrality_scores in the casing table. --- docs/contributing.md | 4 +-- src/squidpy/_constants/_constants.py | 7 ---- src/squidpy/gr/_nhood.py | 52 +++++++++++++--------------- src/squidpy/pl/_graph.py | 3 +- tests/test_string_choices.py | 6 ++++ 5 files changed, 34 insertions(+), 38 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 70d562b0a..7b412d9ea 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -173,9 +173,9 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors - **Parameter bags**: a `*Params` {class}`~typing.TypedDict` in {mod}`squidpy.types`, each key declaring its default as `Annotated[type, Default(value)]`, only when the knobs depend on another argument (one bag per `method`, taken as `method_params`) or several functions share them (taken as `**kwargs: Unpack[XParams]`). Resolve it with `resolve_params` from `squidpy._params`, so unknown keys raise, and keep its keys out of the signature. - **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. - Two exceptions. The first is temporary: arguments still backed by a `ModeEnum` from `squidpy._constants._constants` are case-**sensitive**, because they are handed to `SomeEnum(value)` with no `normalize_choice` in front. As of this writing that is `score` ({func}`~squidpy.gr.centrality_scores`), `shape` ({func}`~squidpy.pl.spatial_scatter`), `corr_axis` and `complex_policy` ({func}`~squidpy.gr.ligrec`), `dendrogram` ({func}`~squidpy.pl.ligrec`), `features` ({func}`~squidpy.im.calculate_image_features`), `method` ({func}`~squidpy.im.process` and {func}`~squidpy.im.segment`), and `dims` ({class}`~squidpy.im.ImageContainer`). Converting the argument to a `Literal` resolved by `normalize_choice` is what fixes it — do that rather than reintroducing a case-folding hook on the enum base class, which would hide how many arguments are still unconverted. + Two exceptions. The first is temporary: arguments still backed by a `ModeEnum` from `squidpy._constants._constants` are case-**sensitive**, because they are handed to `SomeEnum(value)` with no `normalize_choice` in front. As of this writing that is `shape` ({func}`~squidpy.pl.spatial_scatter`), `corr_axis` and `complex_policy` ({func}`~squidpy.gr.ligrec`), `dendrogram` ({func}`~squidpy.pl.ligrec`), `features` ({func}`~squidpy.im.calculate_image_features`), `method` ({func}`~squidpy.im.process` and {func}`~squidpy.im.segment`), and `dims` ({class}`~squidpy.im.ImageContainer`). Converting the argument to a `Literal` resolved by `normalize_choice` is what fixes it — do that rather than reintroducing a case-folding hook on the enum base class, which would hide how many arguments are still unconverted. - The second is permanent: `normalize_choice` needs an option list, and some arguments do not have one until the call runs. {func}`~squidpy.pl.centrality_scores` accepts whichever columns {func}`~squidpy.gr.centrality_scores` happened to write into {attr}`~anndata.AnnData.uns`, so its valid values come from the data, not the signature. Case-fold the user's value against the data where you read it — `{c.casefold(): c for c in columns}` — rather than reaching for `normalize_choice`. + An argument whose valid values are read back from the data, rather than fixed by the signature, still gets a `Literal` when the set that produced them is closed — {func}`~squidpy.pl.centrality_scores` accepts the three measures {func}`~squidpy.gr.centrality_scores` can write, so it declares them. Fold the user's value against the data at the call site only for the genuinely open case, and only to canonicalize; whether a value is *present* is a separate check. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index 436301acb..a672dd266 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -44,13 +44,6 @@ class SegmentationBackend(ModeEnum): CUSTOM = "custom" # callable function -@unique -class Centrality(ModeEnum): - DEGREE = "degree_centrality" - CLUSTERING = "average_clustering" - CLOSENESS = "closeness_centrality" - - @unique class DendrogramAxis(ModeEnum): INTERACTING_MOLS = "interacting_molecules" diff --git a/src/squidpy/gr/_nhood.py b/src/squidpy/gr/_nhood.py index c91abce6a..05827ac44 100644 --- a/src/squidpy/gr/_nhood.py +++ b/src/squidpy/gr/_nhood.py @@ -3,8 +3,8 @@ from __future__ import annotations import warnings -from collections.abc import Iterable, Sequence -from typing import Any, Literal, NamedTuple +from collections.abc import Sequence +from typing import Any, Literal, NamedTuple, get_args import numpy as np import pandas as pd @@ -22,9 +22,8 @@ from spatialdata import SpatialData from squidpy._compat import SKIP_OWN_FRAMES, old_positionals -from squidpy._constants._constants import Centrality from squidpy._constants._pkg_constants import Key -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import ( NDArrayA, RNGLike, @@ -45,6 +44,9 @@ __all__ = ["nhood_enrichment", "NhoodEnrichmentResult", "centrality_scores", "interaction_matrix"] +CentralityScore = Literal["degree_centrality", "average_clustering", "closeness_centrality"] +_CENTRALITY_SCORES = get_args(CentralityScore) + class NhoodEnrichmentResult(NamedTuple): """Result of nhood_enrichment function.""" @@ -546,14 +548,13 @@ def nhood_enrichment( @d.dedent -@inject_docs(c=Centrality) @old_positionals("cluster_key", "score", "connectivity_key", "copy", "n_jobs", "backend", "show_progress_bar") @deprecated_params({"backend": "1.10.0"}) def centrality_scores( adata: AnnData | SpatialData, *, cluster_key: str, - score: str | Iterable[str] | None = None, + score: CentralityScore | Sequence[CentralityScore] | None = None, connectivity_key: str | None = None, copy: bool = False, n_jobs: int | None = None, @@ -578,9 +579,9 @@ def centrality_scores( Group centrality measures as described in :mod:`networkx.algorithms.centrality` :cite:`networkx`. If `None`, use all the options below. Valid options are: - - `{c.CLOSENESS.s!r}` - measure of how close the group is to other nodes. - - `{c.CLUSTERING.s!r}` - measure of the degree to which nodes cluster together. - - `{c.DEGREE.s!r}` - fraction of non-group members connected to group members. + - `'closeness_centrality'` - measure of how close the group is to other nodes. + - `'average_clustering'` - measure of the degree to which nodes cluster together. + - `'degree_centrality'` - fraction of non-group members connected to group members. %(conn_key)s %(copy)s @@ -591,7 +592,7 @@ def centrality_scores( ------- If ``copy = True``, returns a :class:`pandas.DataFrame`. Otherwise, modifies the ``adata`` with the following key: - - :attr:`anndata.AnnData.uns` ``['{{cluster_key}}_centrality_scores']`` - the centrality scores, + - :attr:`anndata.AnnData.uns` ``['{cluster_key}_centrality_scores']`` - the centrality scores, as mentioned above. """ adata = extract_adata_if_sdata(adata, table_key=table_key) @@ -599,18 +600,13 @@ def centrality_scores( _assert_categorical_obs(adata, key=cluster_key) _assert_connectivity_key(adata, key=connectivity_key) - if isinstance(score, str | Centrality): + if score is None: + centrality = list(_CENTRALITY_SCORES) + elif isinstance(score, str): centrality = [score] - elif score is None: - centrality = [c.s for c in Centrality] else: centrality = list(score) - - centralities = [Centrality(c) for c in centrality] - - for c in centralities: - if c not in (Centrality.CLOSENESS, Centrality.DEGREE, Centrality.CLUSTERING): - raise NotImplementedError(f"Centrality `{c}` is not yet implemented.") + centrality = [normalize_choice(c, _CENTRALITY_SCORES, name="score") for c in centrality] # every measure reads the same symmetric, self-loop-free, index-sorted CSR. adj = _symmetric_adjacency(adata.obsp[connectivity_key]) @@ -618,14 +614,14 @@ def centrality_scores( cat = adata.obs[cluster_key].cat.categories.values n_jobs = get_n_numba_threads(n_jobs) - start = logg.info(f"Calculating centralities `{centralities}` using `{n_jobs}` thread(s)") + start = logg.info(f"Calculating centralities `{centrality}` using `{n_jobs}` thread(s)") # cells with a missing label join no group but stay in the graph, so they still count as non-group offsets, members = _group_offsets(adata.obs[cluster_key]) scores: dict[str, NDArrayA] = {} with numba_threads(n_jobs): - if Centrality.CLOSENESS in centralities or Centrality.DEGREE in centralities: + if "closeness_centrality" in centrality or "degree_centrality" in centrality: # one BFS per group yields both measures, so it runs even if only one was asked for with ProgressBar( total=len(cat), unit="group", desc="centrality_scores", disable=not show_progress_bar @@ -633,14 +629,14 @@ def centrality_scores( degree, closeness = _group_degree_closeness( adj.indptr, adj.indices, offsets, members, n_cells, progress ) - if Centrality.DEGREE in centralities: - scores[Centrality.DEGREE.s] = degree - if Centrality.CLOSENESS in centralities: - scores[Centrality.CLOSENESS.s] = closeness - if Centrality.CLUSTERING in centralities: + if "degree_centrality" in centrality: + scores["degree_centrality"] = degree + if "closeness_centrality" in centrality: + scores["closeness_centrality"] = closeness + if "average_clustering" in centrality: # average the per-node clustering coefficients over the group (0 if the group is empty). node_clustering = _local_clustering(adj.indptr, adj.indices, n_cells) - scores[Centrality.CLUSTERING.s] = np.array( + scores["average_clustering"] = np.array( [ float(node_clustering[members[offsets[g] : offsets[g + 1]]].mean()) if offsets[g + 1] > offsets[g] @@ -650,7 +646,7 @@ def centrality_scores( ) # keep the column order the caller asked for, which the measure-by-measure dict above loses. - df = pd.DataFrame({c.s: scores[c.s] for c in centralities}, index=cat) + df = pd.DataFrame({c: scores[c] for c in centrality}, index=cat) if copy: return df diff --git a/src/squidpy/pl/_graph.py b/src/squidpy/pl/_graph.py index 8f6b46c7b..969efdc46 100644 --- a/src/squidpy/pl/_graph.py +++ b/src/squidpy/pl/_graph.py @@ -20,6 +20,7 @@ from squidpy._constants._pkg_constants import Key from squidpy._docs import d from squidpy._validators import assert_non_empty_sequence, get_valid_values, normalize_choice +from squidpy.gr._nhood import CentralityScore from squidpy.gr._utils import _assert_categorical_obs from squidpy.pl._color_utils import Palette_t, _get_palette, _maybe_set_colors from squidpy.pl._utils import _heatmap, save_fig @@ -57,7 +58,7 @@ def centrality_scores( adata: AnnData, *, cluster_key: str, - score: str | Sequence[str] | None = None, + score: CentralityScore | Sequence[CentralityScore] | None = None, legend_kwargs: Mapping[str, Any] = MappingProxyType({}), palette: Palette_t = None, figsize: tuple[float, float] | None = None, diff --git a/tests/test_string_choices.py b/tests/test_string_choices.py index 944776d66..5c8408248 100644 --- a/tests/test_string_choices.py +++ b/tests/test_string_choices.py @@ -57,6 +57,12 @@ def _pl_ripley(adata, mode): "ZERO", "`handle_nan`", ), + "gr.centrality_scores-score": ( + "nhood_data", + lambda a, v: sq.gr.centrality_scores(a, cluster_key="leiden", score=v, copy=True), + "DEGREE_CENTRALITY", + "`score`", + ), "gr.spatial_autocorr-attr": ( "nhood_data", lambda a, v: sq.gr.spatial_autocorr(a, attr=v, genes=a.var_names[:2], copy=True), From ee3fa69871a3fc6fd696ad5d9ec78bc9940b4fe8 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:33:57 +0200 Subject: [PATCH 11/26] Shorten the string-choices convention --- docs/contributing.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 7b412d9ea..c8ec97dea 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -171,11 +171,7 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors - **Keyword-only arguments**: a public function takes its data object (`adata`, `sdata` or `data`) positionally, and every other argument is keyword-only, e.g. `def nhood_enrichment(adata, *, cluster_key, ...)`. Avoid catch-all `**kwargs` that forward whatever they are given unchecked. - **Parameter bags**: a `*Params` {class}`~typing.TypedDict` in {mod}`squidpy.types`, each key declaring its default as `Annotated[type, Default(value)]`, only when the knobs depend on another argument (one bag per `method`, taken as `method_params`) or several functions share them (taken as `**kwargs: Unpack[XParams]`). Resolve it with `resolve_params` from `squidpy._params`, so unknown keys raise, and keep its keys out of the signature. -- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. - - Two exceptions. The first is temporary: arguments still backed by a `ModeEnum` from `squidpy._constants._constants` are case-**sensitive**, because they are handed to `SomeEnum(value)` with no `normalize_choice` in front. As of this writing that is `shape` ({func}`~squidpy.pl.spatial_scatter`), `corr_axis` and `complex_policy` ({func}`~squidpy.gr.ligrec`), `dendrogram` ({func}`~squidpy.pl.ligrec`), `features` ({func}`~squidpy.im.calculate_image_features`), `method` ({func}`~squidpy.im.process` and {func}`~squidpy.im.segment`), and `dims` ({class}`~squidpy.im.ImageContainer`). Converting the argument to a `Literal` resolved by `normalize_choice` is what fixes it — do that rather than reintroducing a case-folding hook on the enum base class, which would hide how many arguments are still unconverted. - - An argument whose valid values are read back from the data, rather than fixed by the signature, still gets a `Literal` when the set that produced them is closed — {func}`~squidpy.pl.centrality_scores` accepts the three measures {func}`~squidpy.gr.centrality_scores` can write, so it declares them. Fold the user's value against the data at the call site only for the genuinely open case, and only to canonicalize; whether a value is *present* is a separate check. +- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Case-sensitive: keys the user chose (obs/obsm keys, layer, channel or cluster names, paths), and arguments still typed as a `ModeEnum` from `squidpy._constants._constants` — convert those to a `Literal` rather than teaching the enum to fold case. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. From 72a7dcfde4c3e6180030b732c4c73f97e000c74a Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:39:18 +0200 Subject: [PATCH 12/26] Give normalize_choice a default for misses Three call sites caught ValueError to mean "no match": two to word their own error, one to leave an unmatched name alone. A `default` keyword says that directly. It takes a sentinel rather than None, since None is itself a legal option. --- src/squidpy/_validators.py | 12 ++++++++++-- src/squidpy/experimental/im/_qc_image.py | 9 +++------ src/squidpy/experimental/tl/_stitched_labels.py | 11 +++++++---- src/squidpy/pl/_graph.py | 5 ++--- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/squidpy/_validators.py b/src/squidpy/_validators.py index b84b59ede..cb0b2e25a 100644 --- a/src/squidpy/_validators.py +++ b/src/squidpy/_validators.py @@ -90,10 +90,16 @@ def assert_isinstance(value: Any, expected_type: type | tuple[type, ...], *, nam raise TypeError(f"Expected `{name}` to be of type `{type_names}`, got `{type(value).__name__}`.") -def normalize_choice(value: Any, options: Iterable[Any], *, name: str) -> Any: +_RAISE = object() +"""Sentinel: `normalize_choice` raises rather than returning a miss.""" + + +def normalize_choice(value: Any, options: Iterable[Any], *, name: str, default: Any = _RAISE) -> Any: """Return the option in *options* matching *value*, ignoring the case of strings. - Raise ValueError naming the valid options if nothing matches. + Raise ValueError naming the valid options if nothing matches, or return *default* + if one was given. `None` is a legal option, so misses are signalled by *default* + alone, never by a `None` return. """ options = list(options) if isinstance(value, str): @@ -102,6 +108,8 @@ def normalize_choice(value: Any, options: Iterable[Any], *, name: str) -> Any: return opt elif value in options: return value + if default is not _RAISE: + return default raise ValueError(f"Expected `{name}` to be one of `{options}`, got `{value!r}`.") diff --git a/src/squidpy/experimental/im/_qc_image.py b/src/squidpy/experimental/im/_qc_image.py index 95f91d4d7..c42d05d36 100644 --- a/src/squidpy/experimental/im/_qc_image.py +++ b/src/squidpy/experimental/im/_qc_image.py @@ -131,12 +131,9 @@ def qc_image( else: metrics = list(metrics) - unknown, resolved = [], [] - for m in metrics: - try: - resolved.append(normalize_choice(m, get_args(QCMetric), name="metrics")) - except ValueError: - unknown.append(m) + matches = [normalize_choice(m, get_args(QCMetric), name="metrics", default=None) for m in metrics] + unknown = [m for m, match in zip(metrics, matches, strict=True) if match is None] + resolved = [match for match in matches if match is not None] if unknown: available = ", ".join(get_args(QCMetric)) raise ValueError(f"Unknown metrics {unknown}. Available: {available}") diff --git a/src/squidpy/experimental/tl/_stitched_labels.py b/src/squidpy/experimental/tl/_stitched_labels.py index cddd3b0d6..1317d1579 100644 --- a/src/squidpy/experimental/tl/_stitched_labels.py +++ b/src/squidpy/experimental/tl/_stitched_labels.py @@ -191,10 +191,13 @@ def _check_strategy(strategy: str | Callable[[pd.Series], object]) -> str | Call """Return a callable unchanged, or the built-in strategy name matching *strategy*.""" if callable(strategy): return strategy - try: - return normalize_choice(strategy, _BUILTIN_STRATEGIES, name="merge_strategy") - except ValueError as e: - raise ValueError(f"{e} Or pass a callable.") from None + match = normalize_choice(strategy, _BUILTIN_STRATEGIES, name="merge_strategy", default=None) + if match is None: + raise ValueError( + f"Expected `merge_strategy` to be one of `{list(_BUILTIN_STRATEGIES)}`, " + f"got `{strategy!r}`. Or pass a callable." + ) + return match # Strategies whose result is always one of the input values, so an integer diff --git a/src/squidpy/pl/_graph.py b/src/squidpy/pl/_graph.py index 969efdc46..7240cf5ef 100644 --- a/src/squidpy/pl/_graph.py +++ b/src/squidpy/pl/_graph.py @@ -102,9 +102,8 @@ def centrality_scores( score = scores if score is None else score score = assert_non_empty_sequence(score, name="centrality scores") - # Match the column names case-insensitively; unknown names stay as-is and get dropped below. - by_fold = {str(s).casefold(): s for s in scores} - score = sorted(get_valid_values([by_fold.get(str(s).casefold(), s) for s in score], scores)) + # Names that match no computed column keep their spelling and get dropped by get_valid_values. + score = sorted(get_valid_values([normalize_choice(s, scores, name="score", default=s) for s in score], scores)) fig, axs = plt.subplots(1, len(score), figsize=figsize, dpi=dpi, constrained_layout=True) axs = np.ravel(axs) # make into iterable From 808eed656e01c23098af77ecd57d747431f350f0 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:43:57 +0200 Subject: [PATCH 13/26] Return the option type from normalize_choice The result is always one of the options, so hand that type back instead of Any; pyright narrows a plain tuple to its literals, so call sites get Literal['F', 'G', 'L'] rather than losing the type. Two overloads because the miss returns the default, whose type is unrelated. Restore the mode option list on pl.nhood_enrichment. --- src/squidpy/_validators.py | 10 +++++++--- src/squidpy/pl/_graph.py | 6 +++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/squidpy/_validators.py b/src/squidpy/_validators.py index cb0b2e25a..e29439f6a 100644 --- a/src/squidpy/_validators.py +++ b/src/squidpy/_validators.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections.abc import Hashable, Iterable, Sequence -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast, overload from squidpy._utils import _unique_order_preserving @@ -94,7 +94,11 @@ def assert_isinstance(value: Any, expected_type: type | tuple[type, ...], *, nam """Sentinel: `normalize_choice` raises rather than returning a miss.""" -def normalize_choice(value: Any, options: Iterable[Any], *, name: str, default: Any = _RAISE) -> Any: +@overload +def normalize_choice[T](value: Any, options: Iterable[T], *, name: str) -> T: ... +@overload +def normalize_choice[T, D](value: Any, options: Iterable[T], *, name: str, default: D) -> T | D: ... +def normalize_choice[T, D](value: Any, options: Iterable[T], *, name: str, default: D | object = _RAISE) -> T | D: """Return the option in *options* matching *value*, ignoring the case of strings. Raise ValueError naming the valid options if nothing matches, or return *default* @@ -109,7 +113,7 @@ def normalize_choice(value: Any, options: Iterable[Any], *, name: str, default: elif value in options: return value if default is not _RAISE: - return default + return cast("D", default) raise ValueError(f"Expected `{name}` to be one of `{options}`, got `{value!r}`.") diff --git a/src/squidpy/pl/_graph.py b/src/squidpy/pl/_graph.py index 7240cf5ef..0cb9fc003 100644 --- a/src/squidpy/pl/_graph.py +++ b/src/squidpy/pl/_graph.py @@ -232,7 +232,11 @@ def nhood_enrichment( %(adata)s %(cluster_key)s mode - Which :func:`squidpy.gr.nhood_enrichment` result to plot. + Which :func:`squidpy.gr.nhood_enrichment` result to plot. Valid options are: + + - `'zscore'` - z-score values of enrichment statistic. + - `'count'` - enrichment count. + %(heatmap_plotting)s kwargs Keyword arguments for :func:`matplotlib.pyplot.text`. From 212db6effe06348f947d58d32f2376660539aad8 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 20:59:08 +0200 Subject: [PATCH 14/26] Collapse the enum error machinery _pretty_raise_enum, ErrorFormatterABC and ABCEnumMeta existed to replace the stdlib's message with one naming the valid options. A _missing_ that raises does the same in three lines: CPython propagates it out of Enum.__call__, so no wrapper, no mixin, no metaclass. Every user of the three was inside this file. The message is byte-identical, which the ligrec and image-feature tests pin. --- src/squidpy/_constants/_utils.py | 57 +++++--------------------------- 1 file changed, 8 insertions(+), 49 deletions(-) diff --git a/src/squidpy/_constants/_utils.py b/src/squidpy/_constants/_utils.py index 865facbc4..d35a6591e 100644 --- a/src/squidpy/_constants/_utils.py +++ b/src/squidpy/_constants/_utils.py @@ -1,45 +1,9 @@ from __future__ import annotations -from abc import ABC, ABCMeta -from collections.abc import Callable, Mapping -from enum import Enum, EnumMeta -from functools import wraps +from enum import Enum from typing import Any -def _pretty_raise_enum(cls: type[ModeEnum], fun: Callable[..., Any]) -> Callable[..., Any]: - @wraps(fun) - def wrapper(*args: Any, **kwargs: Any) -> Any: - try: - return fun(*args, **kwargs) - except ValueError as e: - _cls, value, *_ = args - e.args = (cls._format(value),) - raise e - - if not issubclass(cls, ErrorFormatterABC): - raise TypeError(f"Class `{cls}` must be subtype of `ErrorFormatterABC`.") - elif not len(cls.__members__): - # empty enum, for class hierarchy - return fun - - return wrapper - - -class ErrorFormatterABC(ABC): - """Mixin class that formats invalid value when constructing an enum.""" - - __error_format__ = "Invalid option `{0}` for `{1}`. Valid options are: `{2}`." - - @classmethod - def _format(cls, value: Enum) -> str: - return cls.__error_format__.format( - value, - cls.__name__, - [m.value for m in cls.__members__.values()], # type: ignore[attr-defined] - ) - - class PrettyEnum(Enum): """Enum with a pretty __str__ and __repr__.""" @@ -50,21 +14,16 @@ def __str__(self) -> str: return str(self.value) -class ABCEnumMeta(EnumMeta, ABCMeta): - """Metaclass which injects.""" - - def __new__( # noqa: D102 - cls, clsname: str, bases: tuple[EnumMeta, ...], namespace: Mapping[str, Any] - ) -> ABCEnumMeta: - res = super().__new__(cls, clsname, bases, namespace) # type: ignore[arg-type] - res.__new__ = _pretty_raise_enum(res, res.__new__) # type: ignore[method-assign,arg-type] - return res - - # TODO(michalk8): members now compare equal to their value; `.s` is redundant at its 63 call sites. -class ModeEnum(str, ErrorFormatterABC, PrettyEnum, metaclass=ABCEnumMeta): +class ModeEnum(str, PrettyEnum): """Enum which prints available values when invalid value has been passed.""" + @classmethod + def _missing_(cls, value: object) -> None: + raise ValueError( + f"Invalid option `{value}` for `{cls.__name__}`. Valid options are: `{[m.value for m in cls]}`." + ) + @property def s(self) -> str: """Return the :attr:`value` as :class:`str`.""" From f2e3b46c1aabdeb146f9ab40f729d0f9a825bf89 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:04:22 +0200 Subject: [PATCH 15/26] Annotate _missing_ as NoReturn --- src/squidpy/_constants/_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/squidpy/_constants/_utils.py b/src/squidpy/_constants/_utils.py index d35a6591e..56e5415af 100644 --- a/src/squidpy/_constants/_utils.py +++ b/src/squidpy/_constants/_utils.py @@ -1,7 +1,7 @@ from __future__ import annotations from enum import Enum -from typing import Any +from typing import Any, NoReturn class PrettyEnum(Enum): @@ -14,16 +14,16 @@ def __str__(self) -> str: return str(self.value) -# TODO(michalk8): members now compare equal to their value; `.s` is redundant at its 63 call sites. class ModeEnum(str, PrettyEnum): """Enum which prints available values when invalid value has been passed.""" @classmethod - def _missing_(cls, value: object) -> None: + def _missing_(cls, value: object) -> NoReturn: raise ValueError( f"Invalid option `{value}` for `{cls.__name__}`. Valid options are: `{[m.value for m in cls]}`." ) + # TODO(michalk8): members now compare equal to their value, so `.s` is redundant at its 63 call sites. @property def s(self) -> str: """Return the :attr:`value` as :class:`str`.""" From 396c783264ebe981781ce13026c6c2338693a117 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:08:51 +0200 Subject: [PATCH 16/26] Convert dendrogram and drop NicheDefinitions --- src/squidpy/_constants/_constants.py | 15 -------- src/squidpy/gr/_niche.py | 54 ++++++++++++++-------------- src/squidpy/pl/_ligrec.py | 27 +++++++------- 3 files changed, 38 insertions(+), 58 deletions(-) diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index a672dd266..13fe5e3a0 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -44,13 +44,6 @@ class SegmentationBackend(ModeEnum): CUSTOM = "custom" # callable function -@unique -class DendrogramAxis(ModeEnum): - INTERACTING_MOLS = "interacting_molecules" - INTERACTING_CLUSTERS = "interacting_clusters" - BOTH = "both" - - @unique class InferDimensions(ModeEnum): DEFAULT = "default" @@ -65,11 +58,3 @@ class ScatterShape(ModeEnum): HEX = "hex" -@unique -class NicheDefinitions(ModeEnum): - NEIGHBORHOOD = "neighborhood" - UTAG = "utag" - CELLCHARTER = "cellcharter" - SPATIALLEIDEN = "spatialleiden" - SPOT = "spot" - BANKSY = "banksy" diff --git a/src/squidpy/gr/_niche.py b/src/squidpy/gr/_niche.py index c27210972..7b42ca87b 100644 --- a/src/squidpy/gr/_niche.py +++ b/src/squidpy/gr/_niche.py @@ -19,8 +19,7 @@ from spatialdata import SpatialData, sanitize_table from spatialdata._logging import logger as logg -from squidpy._constants._constants import NicheDefinitions -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import ( RNGLike, SeedLike, @@ -47,7 +46,6 @@ @d.dedent -@inject_docs(fla=NicheDefinitions) @deprecated_randomness_param def calculate_niche( # noqa: PLR0917, deprecated and removed in v1.9.0 data: AnnData | SpatialData, @@ -100,10 +98,10 @@ def calculate_niche( # noqa: PLR0917, deprecated and removed in v1.9.0 %(adata)s flavor Method to use for niche calculation. Available options are: - - `{fla.NEIGHBORHOOD.s!r}` - cluster the neighborhood profile. - - `{fla.UTAG.s!r}` - use utag algorithm (matrix multiplication). - - `{fla.SPATIALLEIDEN.s!r}` - cluster spatially resolved omics data using Multiplex Leiden. - - `{fla.CELLCHARTER.s!r}` - a simplified version of CellCharter's approach, using PCA for dimensionality reduction. An arbitrary embedding can be used instead of PCA by setting the `use_rep` parameter which will try to find the embedding in `adata.obsm`. + - `'neighborhood'` - cluster the neighborhood profile. + - `'utag'` - use utag algorithm (matrix multiplication). + - `'spatialleiden'` - cluster spatially resolved omics data using Multiplex Leiden. + - `'cellcharter'` - a simplified version of CellCharter's approach, using PCA for dimensionality reduction. An arbitrary embedding can be used instead of PCA by setting the `use_rep` parameter which will try to find the embedding in `adata.obsm`. library_key Deprecated, and removed with this function in v1.9.0; no flavor function takes it. Each library is fitted on its own and its labels are prefixed ``lib=_``, so a niche in one @@ -117,69 +115,69 @@ def calculate_niche( # noqa: PLR0917, deprecated and removed in v1.9.0 for lib in adata.obs["library"].unique(): sub = adata[adata.obs["library"] == lib].copy() sq.gr.calculate_niche_utag(sub, resolutions=0.5) - adata.obs.loc[sub.obs_names, "niche"] = f"lib={{lib}}_" + sub.obs["utag_niche_res=0.5"].astype(str) + adata.obs.loc[sub.obs_names, "niche"] = f"lib={lib}_" + sub.obs["utag_niche_res=0.5"].astype(str) %(table_key)s mask Boolean array to filter cells which won't get assigned to a niche. Spelled ``cluster_mask`` on the three flavors that build an embedding, and - `{fla.SPATIALLEIDEN.s!r}` raises rather than accepting one it cannot honour. + `'spatialleiden'` raises rather than accepting one it cannot honour. Note that if you want to exclude these cells during neighborhood calculation already, you should subset your AnnData table before running 'sq.gr.spatial_neigbors'. Mask can look like the following. Here, the index values would correspond to adata.obs.index. The entries that are False are the ones ignored. mask = Series([False, False, True], index = ["a", "b", "c"]) groups Groups based on which to calculate neighborhood profile (E.g. columns of cell type annotations in adata.obs). - Required if flavor == `{fla.NEIGHBORHOOD.s!r}`. + Required if flavor == `'neighborhood'`. n_neighbors Number of neighbors to use for 'scanpy.pp.neighbors' before clustering using leiden algorithm. - Required if flavor == `{fla.NEIGHBORHOOD.s!r}` or flavor == `{fla.UTAG.s!r}`. + Required if flavor == `'neighborhood'` or flavor == `'utag'`. resolutions List of resolutions to use for leiden clustering. In the case of spatialleiden you can pass a tuple. Resolution for the latent space and spatial layer, respectively. A single float applies to both layers. - Required if flavor == `{fla.NEIGHBORHOOD.s!r}` or flavor == `{fla.UTAG.s!r}`. - Optional if flavor == `{fla.SPATIALLEIDEN.s!r}`. + Required if flavor == `'neighborhood'` or flavor == `'utag'`. + Optional if flavor == `'spatialleiden'`. min_niche_size Minimum required size of a niche. Niches with fewer cells will be labeled as 'not_a_niche'. - Optional if flavor == `{fla.NEIGHBORHOOD.s!r}`. + Optional if flavor == `'neighborhood'`. scale If 'True', compute z-scores of neighborhood profiles. - Optional if flavor == `{fla.NEIGHBORHOOD.s!r}`. + Optional if flavor == `'neighborhood'`. abs_nhood If 'True', calculate niches based on absolute neighborhood profile. - Optional if flavor == `{fla.NEIGHBORHOOD.s!r}`. + Optional if flavor == `'neighborhood'`. distance n-hop neighbor adjacency matrices to use e.g. [1,2,3] for 1-hop,2-hop,3-hop neighbors respectively or "5" for 1-hop,...,5-hop neighbors. 0 (self) is always included. - Required if flavor == `{fla.CELLCHARTER.s!r}`. - Optional if flavor == `{fla.NEIGHBORHOOD.s!r}`. + Required if flavor == `'cellcharter'`. + Optional if flavor == `'neighborhood'`. n_hop_weights How to weight subsequent n-hop adjacency matrices. E.g. [1, 0.5, 0.25] for weights of 1-hop, 2-hop, 3-hop adjacency matrices respectively. - Optional if flavor == `{fla.NEIGHBORHOOD.s!r}` and `distance` > 1. + Optional if flavor == `'neighborhood'` and `distance` > 1. aggregation How to aggregate count matrices. Either 'mean' or 'variance'. - Required if flavor == `{fla.CELLCHARTER.s!r}`. + Required if flavor == `'cellcharter'`. n_components Number of components to use for GMM. - Required if flavor == `{fla.CELLCHARTER.s!r}`. + Required if flavor == `'cellcharter'`. %(rng)s - Optional if flavor == `{fla.CELLCHARTER.s!r}` or flavor == `{fla.SPATIALLEIDEN.s!r}`. + Optional if flavor == `'cellcharter'` or flavor == `'spatialleiden'`. spatial_connectivities_key Key in `adata.obsp` where spatial connectivities are stored. - Required if flavor == `{fla.SPATIALLEIDEN.s!r}`. + Required if flavor == `'spatialleiden'`. latent_connectivities_key Key in `adata.obsp` where gene expression connectivities are stored. - Required if flavor == `{fla.SPATIALLEIDEN.s!r}`. + Required if flavor == `'spatialleiden'`. layer_ratio The ratio of the weighting of the layers; latent space vs spatial. A higher ratio will increase relevance of the spatial neighbors and lead to more spatially homogeneous clusters. - Optional if flavor == `{fla.SPATIALLEIDEN.s!r}`. + Optional if flavor == `'spatialleiden'`. n_iterations Number of iterations to run the Leiden algorithm. If the number is negative it runs until convergence. - Optional if flavor == `{fla.SPATIALLEIDEN.s!r}`. + Optional if flavor == `'spatialleiden'`. use_weights Whether to use weights for the edges for latent space and spatial neighbors, respectively. A single bool applies to both layers. - Optional if flavor == `{fla.SPATIALLEIDEN.s!r}`. + Optional if flavor == `'spatialleiden'`. use_rep Key in `adata.obsm` where the embedding is stored. If provided, this embedding will be used instead of PCA for dimensionality reduction. - Optional if flavor == `{fla.CELLCHARTER.s!r}`. + Optional if flavor == `'cellcharter'`. inplace If 'True', perform the operation in place. If 'False', return a new AnnData object with the niche labels. diff --git a/src/squidpy/pl/_ligrec.py b/src/squidpy/pl/_ligrec.py index ff5ed1744..cde67bc97 100644 --- a/src/squidpy/pl/_ligrec.py +++ b/src/squidpy/pl/_ligrec.py @@ -2,7 +2,7 @@ from collections.abc import Mapping, Sequence from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import Any, Literal import matplotlib.pyplot as plt import numpy as np @@ -15,10 +15,10 @@ from scipy.cluster import hierarchy as sch from squidpy._compat import old_positionals -from squidpy._constants._constants import DendrogramAxis from squidpy._constants._pkg_constants import Key from squidpy._docs import d from squidpy._utils import _unique_order_preserving, verbosity +from squidpy._validators import normalize_choice from squidpy.pl._utils import _dendrogram, _filter_kwargs, save_fig __all__ = ["ligrec"] @@ -156,7 +156,7 @@ def ligrec( pvalue_threshold: float = 1.0, remove_empty_interactions: bool = True, remove_nonsig_interactions: bool = False, - dendrogram: str | None = None, + dendrogram: Literal["interacting_molecules", "interacting_clusters", "both"] | None = None, alpha: float | None = 0.001, swap_axes: bool = False, title: str | None = None, @@ -254,9 +254,9 @@ def get_dendrogram(adata: AnnData, linkage: str = "complete") -> Mapping[str, An } if dendrogram is not None: - dendrogram = DendrogramAxis(dendrogram) # type: ignore[assignment] - if TYPE_CHECKING: - assert isinstance(dendrogram, DendrogramAxis) + dendrogram = normalize_choice( + dendrogram, ("interacting_molecules", "interacting_clusters", "both"), name="dendrogram" + ) if isinstance(adata, AnnData): if cluster_key is None: @@ -318,7 +318,7 @@ def get_dendrogram(adata: AnnData, linkage: str = "complete") -> Mapping[str, An start, label_ranges = 0, {} - if dendrogram == DendrogramAxis.INTERACTING_CLUSTERS: + if dendrogram == "interacting_clusters": # rows are now cluster combinations, not interacting pairs pvals = pvals.T means = means.T @@ -349,7 +349,7 @@ def get_dendrogram(adata: AnnData, linkage: str = "complete") -> Mapping[str, An adata.X = (adata.X - minn) / delta try: - if dendrogram == DendrogramAxis.BOTH: + if dendrogram == "both": row_order, col_order, _, _ = _dendrogram( adata.X, method="complete", @@ -384,8 +384,8 @@ def get_dendrogram(adata: AnnData, linkage: str = "complete") -> Mapping[str, An dot_color_df=means, dot_size_df=pvals, title=title, - var_group_labels=(None if dendrogram == DendrogramAxis.BOTH else list(label_ranges.keys())), - var_group_positions=(None if dendrogram == DendrogramAxis.BOTH else list(label_ranges.values())), + var_group_labels=(None if dendrogram == "both" else list(label_ranges.keys())), + var_group_positions=(None if dendrogram == "both" else list(label_ranges.values())), standard_scale=None, figsize=figsize, ) @@ -398,10 +398,7 @@ def get_dendrogram(adata: AnnData, linkage: str = "complete") -> Mapping[str, An **_filter_kwargs(sc.pl.DotPlot.legend, kwargs), ) ) - if dendrogram in ( - DendrogramAxis.INTERACTING_MOLS, - DendrogramAxis.INTERACTING_CLUSTERS, - ): + if dendrogram in ("interacting_molecules", "interacting_clusters"): # ignore the warning about mismatching groups with verbosity(0): dp.add_dendrogram(size=1.6, dendrogram_key="dendrogram") @@ -410,7 +407,7 @@ def get_dendrogram(adata: AnnData, linkage: str = "complete") -> Mapping[str, An dp.make_figure() - if dendrogram != DendrogramAxis.BOTH: + if dendrogram != "both": # remove the target part in: source | target labs = dp.ax_dict["mainplot_ax"].get_yticklabels() if swap_axes else dp.ax_dict["mainplot_ax"].get_xticklabels() for text in labs: From 86d356a9d1a3db902f89cb8f4651308e958d4178 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:09:03 +0000 Subject: [PATCH 17/26] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/squidpy/_constants/_constants.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index 13fe5e3a0..0809b58b7 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -56,5 +56,3 @@ class ScatterShape(ModeEnum): CIRCLE = "circle" SQUARE = "square" HEX = "hex" - - From 6f338f6dc064cc115261b24a0ff14b300317c24b Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:11:49 +0200 Subject: [PATCH 18/26] Convert corr_axis and complex_policy --- src/squidpy/_constants/_constants.py | 12 ------ src/squidpy/gr/_ligrec.py | 55 +++++++++++++--------------- tests/graph/test_ligrec.py | 4 +- 3 files changed, 28 insertions(+), 43 deletions(-) diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index 0809b58b7..87518db90 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -17,18 +17,6 @@ class ImageFeature(ModeEnum): # _ligrec.py -@unique -class CorrAxis(ModeEnum): - INTERACTIONS = "interactions" - CLUSTERS = "clusters" - - -@unique -class ComplexPolicy(ModeEnum): - MIN = "min" - ALL = "all" - - @unique class Processing(ModeEnum): SMOOTH = "smooth" diff --git a/src/squidpy/gr/_ligrec.py b/src/squidpy/gr/_ligrec.py index 2ed1f38ac..b2412c2e6 100644 --- a/src/squidpy/gr/_ligrec.py +++ b/src/squidpy/gr/_ligrec.py @@ -20,7 +20,6 @@ from spatialdata import SpatialData from squidpy._compat import old_positionals -from squidpy._constants._constants import ComplexPolicy, CorrAxis from squidpy._constants._pkg_constants import Key from squidpy._docs import d, inject_docs from squidpy._utils import ( @@ -32,7 +31,7 @@ get_n_numba_threads, numba_threads, ) -from squidpy._validators import assert_positive, check_tuple_needles +from squidpy._validators import assert_positive, check_tuple_needles, normalize_choice from squidpy.gr._utils import ( _assert_categorical_obs, _genesymbols, @@ -64,7 +63,7 @@ class TempResult(NamedTuple): def _fdr_correct( pvals: pd.DataFrame, corr_method: str, - corr_axis: Literal["interactions", "clusters"] | CorrAxis, + corr_axis: Literal["interactions", "clusters"], alpha: float = 0.05, ) -> pd.DataFrame: """Correct p-values for FDR along specific axis in ``pvals``.""" @@ -83,12 +82,12 @@ def fdr(pvals: pd.Series) -> SparseArray: return SparseArray(qvals, dtype=qvals.dtype, fill_value=np.nan) - corr_axis = CorrAxis(corr_axis) + corr_axis = normalize_choice(corr_axis, ("interactions", "clusters"), name="corr_axis") - if corr_axis == CorrAxis.CLUSTERS: + if corr_axis == "clusters": # clusters are in columns pvals = pvals.apply(fdr) - elif corr_axis == CorrAxis.INTERACTIONS: + elif corr_axis == "interactions": pvals = pvals.T.apply(fdr).T else: raise NotImplementedError(f"FDR correction for `{corr_axis}` is not implemented.") @@ -140,9 +139,9 @@ def __init__(self, adata: AnnData, use_raw: bool = True): @d.get_full_description(base="PT_prepare") @d.get_sections(base="PT_prepare", sections=["Parameters", "Returns"]) - @inject_docs(src=SOURCE, tgt=TARGET, cp=ComplexPolicy) + @inject_docs(src=SOURCE, tgt=TARGET) def prepare( - self, interactions: Interaction_t, complex_policy: Literal["min", "all"] | ComplexPolicy = ComplexPolicy.MIN.v + self, interactions: Interaction_t, complex_policy: Literal["min", "all"] = "min" ) -> PermutationTestABC: """ Prepare self for running the permutation test. @@ -162,9 +161,9 @@ def prepare( complex_policy Policy on how to handle complexes. Valid options are: - - `{cp.MIN.s!r}` - select gene with the minimum average expression. This is the same as in + - `'min'` - select gene with the minimum average expression. This is the same as in :cite:`cellphonedb`. - - `{cp.ALL.s!r}` - select all possible combinations between `{src!r}` and `{tgt!r}` complexes. + - `'all'` - select all possible combinations between `{src!r}` and `{tgt!r}` complexes. Returns ------- @@ -172,7 +171,7 @@ def prepare( - :attr:`interactions` - filtered interactions whose `{src!r}` and `{tgt!r}` are both in the data. """ - complex_policy = ComplexPolicy(complex_policy) + complex_policy = normalize_choice(complex_policy, ("min", "all"), name="complex_policy") if isinstance(interactions, Mapping): interactions = pd.DataFrame(interactions) @@ -235,7 +234,7 @@ def prepare( @d.get_full_description(base="PT_test") @d.get_sections(base="PT_test", sections=["Parameters"]) @d.dedent - @inject_docs(src=SOURCE, tgt=TARGET, fa=CorrAxis) + @inject_docs(src=SOURCE, tgt=TARGET) @deprecated_params({"numba_parallel": "1.10.0", "backend": "1.10.0"}) @deprecated_randomness_param def test( @@ -247,7 +246,7 @@ def test( threshold: float = 0.01, rng: SeedLike | RNGLike | None = None, corr_method: str | None = None, - corr_axis: Literal["interactions", "clusters"] | CorrAxis = CorrAxis.INTERACTIONS.v, + corr_axis: Literal["interactions", "clusters"] = "interactions", alpha: float = 0.05, copy: bool = False, key_added: str | None = None, @@ -272,8 +271,8 @@ def test( corr_axis Axis over which to perform the FDR correction. Only used when ``corr_method != None``. Valid options are: - - `{fa.INTERACTIONS.s!r}` - correct interactions by performing FDR correction across the clusters. - - `{fa.CLUSTERS.s!r}` - correct clusters by performing FDR correction across the interactions. + - `'interactions'` - correct interactions by performing FDR correction across the clusters. + - `'clusters'` - correct clusters by performing FDR correction across the interactions. alpha Significance level for FDR correction. Only used when ``corr_method != None``. %(copy)s @@ -294,9 +293,7 @@ def test( _assert_categorical_obs(self._adata, key=cluster_key) if corr_method is not None: - corr_axis = CorrAxis(corr_axis) - if TYPE_CHECKING: - assert isinstance(corr_axis, CorrAxis) + corr_axis = normalize_choice(corr_axis, ("interactions", "clusters"), name="corr_axis") if len(self._adata.obs[cluster_key].cat.categories) <= 1: raise ValueError( @@ -369,7 +366,7 @@ def test( if corr_method is not None: logg.info( - f"Performing FDR correction across the `{corr_axis.v}` using method `{corr_method}` at level `{alpha}`" + f"Performing FDR correction across the `{corr_axis}` using method `{corr_method}` at level `{alpha}`" ) res["pvalues"] = _fdr_correct(res["pvalues"], corr_method, corr_axis, alpha=alpha) @@ -402,8 +399,8 @@ def _filter_interactions_by_genes(self) -> None: if self.interactions.empty: raise ValueError("After filtering by genes, no interactions remain.") - @inject_docs(src=SOURCE, tgt=TARGET, cp=ComplexPolicy) - def _filter_interactions_complexes(self, complex_policy: ComplexPolicy) -> None: + @inject_docs(src=SOURCE, tgt=TARGET) + def _filter_interactions_complexes(self, complex_policy: Literal["min", "all"]) -> None: """ Filter the :attr:`interactions` by extracting genes from complexes. @@ -412,9 +409,9 @@ def _filter_interactions_complexes(self, complex_policy: ComplexPolicy) -> None: complex_policy Policy on how to handle complexes. Valid options are: - - `{cp.MIN.s!r}` - select gene with the minimum average expression. This is the same as in + - `'min'` - select gene with the minimum average expression. This is the same as in :cite:`cellphonedb`. - - `{cp.ALL.s!r}` - select all possible combinations between `{src!r}` and `{tgt!r}` complexes. + - `'all'` - select all possible combinations between `{src!r}` and `{tgt!r}` complexes. Returns ------- @@ -422,7 +419,7 @@ def _filter_interactions_complexes(self, complex_policy: ComplexPolicy) -> None: - :attr:`interactions` - filtered interactions whose `{src!r}` and `{tgt!r}` are both in the data. - Note that for ``complex_policy={cp.ALL.s!r}``, all pairwise comparisons within a complex are created, + Note that for ``complex_policy='all'``, all pairwise comparisons within a complex are created, but no filtering happens at this stage - genes not present in the data are filtered at a later stage. """ @@ -451,11 +448,11 @@ def find_min_gene_in_complex(_complex: str | None) -> str | None: assert isinstance(self._interactions, pd.DataFrame) assert isinstance(self.interactions, pd.DataFrame) - if complex_policy == ComplexPolicy.MIN: + if complex_policy == "min": logg.debug("DEBUG: Selecting genes from complexes based on minimum average expression") self.interactions[SOURCE] = self.interactions[SOURCE].apply(find_min_gene_in_complex) self.interactions[TARGET] = self.interactions[TARGET].apply(find_min_gene_in_complex) - elif complex_policy == ComplexPolicy.ALL: + elif complex_policy == "all": logg.debug("DEBUG: Creating all gene combinations within complexes") src = self.interactions.pop(SOURCE).apply(lambda s: str(s).split("_")).explode() src.name = SOURCE @@ -497,7 +494,7 @@ class PermutationTest(PermutationTestABC): def prepare( self, interactions: Interaction_t | None = None, - complex_policy: Literal["min", "all"] = ComplexPolicy.MIN.v, + complex_policy: Literal["min", "all"] = "min", interactions_params: Mapping[str, Any] = MappingProxyType({}), transmitter_params: Mapping[str, Any] = MappingProxyType({"categories": "ligand"}), receiver_params: Mapping[str, Any] = MappingProxyType({"categories": "receptor"}), @@ -574,10 +571,10 @@ def ligrec( *, cluster_key: str, interactions: Interaction_t | None = None, - complex_policy: Literal["min", "all"] = ComplexPolicy.MIN.v, + complex_policy: Literal["min", "all"] = "min", threshold: float = 0.01, corr_method: str | None = None, - corr_axis: Literal["interactions", "clusters"] = CorrAxis.CLUSTERS.v, + corr_axis: Literal["interactions", "clusters"] = "clusters", use_raw: bool = True, copy: bool = False, key_added: str | None = None, diff --git a/tests/graph/test_ligrec.py b/tests/graph/test_ligrec.py index f888980b0..91676412a 100644 --- a/tests/graph/test_ligrec.py +++ b/tests/graph/test_ligrec.py @@ -49,11 +49,11 @@ def test_only_1_cluster(self, adata: AnnData, interactions: Interactions_t): ligrec(adata, cluster_key="foo", interactions=interactions) def test_invalid_complex_policy(self, adata: AnnData, interactions: Interactions_t): - with pytest.raises(ValueError, match=r"Invalid option `foobar` for `ComplexPolicy`."): + with pytest.raises(ValueError, match=r"Expected `complex_policy` to be one of"): ligrec(adata, cluster_key=_CK, interactions=interactions, complex_policy="foobar") def test_invalid_fdr_axis(self, adata: AnnData, interactions: Interactions_t): - with pytest.raises(ValueError, match=r"Invalid option `foobar` for `CorrAxis`."): + with pytest.raises(ValueError, match=r"Expected `corr_axis` to be one of"): ligrec(adata, cluster_key=_CK, interactions=interactions, corr_axis="foobar", corr_method="fdr_bh") def test_too_few_permutations(self, adata: AnnData, interactions: Interactions_t): From 9a5896c1b40d69144e382b447f865be25f43c2cd Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:12:28 +0200 Subject: [PATCH 19/26] Convert corr_axis and complex_policy --- src/squidpy/gr/_ligrec.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/squidpy/gr/_ligrec.py b/src/squidpy/gr/_ligrec.py index b2412c2e6..c8f96c299 100644 --- a/src/squidpy/gr/_ligrec.py +++ b/src/squidpy/gr/_ligrec.py @@ -140,9 +140,7 @@ def __init__(self, adata: AnnData, use_raw: bool = True): @d.get_full_description(base="PT_prepare") @d.get_sections(base="PT_prepare", sections=["Parameters", "Returns"]) @inject_docs(src=SOURCE, tgt=TARGET) - def prepare( - self, interactions: Interaction_t, complex_policy: Literal["min", "all"] = "min" - ) -> PermutationTestABC: + def prepare(self, interactions: Interaction_t, complex_policy: Literal["min", "all"] = "min") -> PermutationTestABC: """ Prepare self for running the permutation test. From ed27b26c98f08ece7e599dccd60ae849cea2b444 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:16:00 +0200 Subject: [PATCH 20/26] Convert process and segment methods Both arguments also take a callable, so normalize_choice sits behind the existing isinstance guard and the callable path is untouched. segment keeps accepting "custom" so passing it as a string still raises the TypeError about needing a callable, rather than turning into a ValueError. Key.img.segment and Key.img.process lose their enum round-trip, and the qc_image metric check drops a comprehension. --- src/squidpy/_constants/_constants.py | 15 ------------ src/squidpy/_constants/_pkg_constants.py | 14 +++-------- src/squidpy/experimental/im/_qc_image.py | 9 +++---- src/squidpy/im/_process.py | 27 ++++++++++----------- src/squidpy/im/_segment.py | 31 +++++++++++++----------- tests/image/test_segmentation.py | 3 +-- 6 files changed, 38 insertions(+), 61 deletions(-) diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index 87518db90..d75a9d9ad 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -17,21 +17,6 @@ class ImageFeature(ModeEnum): # _ligrec.py -@unique -class Processing(ModeEnum): - SMOOTH = "smooth" - GRAY = "gray" - - -@unique -class SegmentationBackend(ModeEnum): - LOG = "log" - DOG = "dog" - DOH = "doh" - WATERSHED = "watershed" - CUSTOM = "custom" # callable function - - @unique class InferDimensions(ModeEnum): DEFAULT = "default" diff --git a/src/squidpy/_constants/_pkg_constants.py b/src/squidpy/_constants/_pkg_constants.py index 924e6240b..8c5958585 100644 --- a/src/squidpy/_constants/_pkg_constants.py +++ b/src/squidpy/_constants/_pkg_constants.py @@ -7,8 +7,6 @@ from anndata import AnnData -from squidpy._constants._constants import Processing, SegmentationBackend - _SEP = "_" @@ -23,18 +21,14 @@ def __get__(self, obj: Any, owner: Any) -> str: class Key: class img: @classmethod - def segment(cls, backend: str | SegmentationBackend, layer_added: str | None = None) -> str: - return f"segmented_{SegmentationBackend(backend).s}" if layer_added is None else layer_added + def segment(cls, backend: str, layer_added: str | None = None) -> str: + return f"segmented_{backend}" if layer_added is None else layer_added @classmethod - def process( - cls, method: str | Processing | Callable[[Any], Any], img_id: str, layer_added: str | None = None - ) -> str: + def process(cls, method: str | Callable[[Any], Any], img_id: str, layer_added: str | None = None) -> str: if layer_added is not None: return layer_added - if isinstance(method, Processing): - method = method.s - elif callable(method): + if callable(method): method = getattr(method, "__name__", "custom") return f"{img_id}_{method}" diff --git a/src/squidpy/experimental/im/_qc_image.py b/src/squidpy/experimental/im/_qc_image.py index c42d05d36..b4ca49585 100644 --- a/src/squidpy/experimental/im/_qc_image.py +++ b/src/squidpy/experimental/im/_qc_image.py @@ -131,12 +131,9 @@ def qc_image( else: metrics = list(metrics) - matches = [normalize_choice(m, get_args(QCMetric), name="metrics", default=None) for m in metrics] - unknown = [m for m, match in zip(metrics, matches, strict=True) if match is None] - resolved = [match for match in matches if match is not None] - if unknown: - available = ", ".join(get_args(QCMetric)) - raise ValueError(f"Unknown metrics {unknown}. Available: {available}") + resolved = [normalize_choice(m, get_args(QCMetric), name="metrics", default=None) for m in metrics] + if unknown := [m for m, match in zip(metrics, resolved, strict=True) if match is None]: + raise ValueError(f"Unknown metrics {unknown}. Available: {', '.join(get_args(QCMetric))}") metrics = resolved # Validate H&E constraint diff --git a/src/squidpy/im/_process.py b/src/squidpy/im/_process.py index 52c9b73d1..7ca060ddb 100644 --- a/src/squidpy/im/_process.py +++ b/src/squidpy/im/_process.py @@ -2,7 +2,7 @@ from collections.abc import Callable, Mapping, Sequence from types import MappingProxyType -from typing import Any +from typing import Any, Literal import dask.array as da from dask_image.ndfilters import gaussian_filter as dask_gf @@ -10,17 +10,16 @@ from scipy.ndimage import gaussian_filter as scipy_gf from squidpy._compat import old_positionals -from squidpy._constants._constants import Processing from squidpy._constants._pkg_constants import Key -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import NDArrayA +from squidpy._validators import normalize_choice from squidpy.im._container import ImageContainer __all__ = ["process"] @d.dedent -@inject_docs(p=Processing) @old_positionals( "layer", "library_id", "method", "chunks", "lazy", "layer_added", "channel_dim", "copy", "apply_kwargs" ) @@ -29,7 +28,7 @@ def process( *, layer: str | None = None, library_id: str | Sequence[str] | None = None, - method: str | Callable[..., NDArrayA] = "smooth", + method: Literal["smooth", "gray"] | Callable[..., NDArrayA] = "smooth", chunks: int | None = None, lazy: bool = False, layer_added: str | None = None, @@ -50,16 +49,16 @@ def process( method Processing method to use. Valid options are: - - `{p.SMOOTH.s!r}` - :func:`skimage.filters.gaussian`. - - `{p.GRAY.s!r}` - :func:`skimage.color.rgb2gray`. + - `'smooth'` - :func:`skimage.filters.gaussian`. + - `'gray'` - :func:`skimage.color.rgb2gray`. %(custom_fn)s %(chunks_lazy)s %(layer_added)s - If `None`, use ``'{{layer}}_{{method}}'``. + If `None`, use ``'{layer}_{method}'``. channel_dim Name of the channel dimension of the new image layer. Default is the same as the original, if the - processing function does not change the number of channels, and ``'{{channel}}_{{processing}}'`` otherwise. + processing function does not change the number of channels, and ``'{channel}_{processing}'`` otherwise. %(copy_cont)s apply_kwargs Keyword arguments for :meth:`squidpy.im.ImageContainer.apply`. @@ -68,11 +67,11 @@ def process( Returns ------- - If ``copy = True``, returns a new container with the processed image in ``'{{layer_added}}'``. + If ``copy = True``, returns a new container with the processed image in ``'{layer_added}'``. Otherwise, modifies the ``img`` with the following key: - - :class:`squidpy.im.ImageContainer` ``['{{layer_added}}']`` - the processed image. + - :class:`squidpy.im.ImageContainer` ``['{layer_added}']`` - the processed image. Raises ------ @@ -82,7 +81,7 @@ def process( from squidpy.pl._utils import _to_grayscale layer = img._get_layer(layer) - method = Processing(method) if isinstance(method, str | Processing) else method # type: ignore[assignment] + method = normalize_choice(method, ("smooth", "gray"), name="method") if isinstance(method, str) else method apply_kwargs = dict(apply_kwargs) apply_kwargs["lazy"] = lazy @@ -92,7 +91,7 @@ def process( if callable(method): callback = method - elif method == Processing.SMOOTH: # type: ignore[comparison-overlap] + elif method == "smooth": if library_id is None: expected_ndim = 4 kwargs.setdefault("sigma", [1, 1, 0, 0]) # y, x, z, c @@ -112,7 +111,7 @@ def process( callback = lambda arr, **kwargs: dask_gf(da.asarray(arr).rechunk(chunks_), **kwargs) # noqa: E731 else: callback = scipy_gf - elif method == Processing.GRAY: # type: ignore[comparison-overlap] + elif method == "gray": apply_kwargs["drop_axis"] = 3 callback = _to_grayscale else: diff --git a/src/squidpy/im/_segment.py b/src/squidpy/im/_segment.py index cc9403b8e..31e468541 100644 --- a/src/squidpy/im/_segment.py +++ b/src/squidpy/im/_segment.py @@ -3,7 +3,7 @@ from abc import ABC, abstractmethod from collections.abc import Callable, Mapping, Sequence from types import MappingProxyType -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Literal import dask.array as da import numpy as np @@ -13,10 +13,10 @@ from skimage.filters import threshold_otsu from skimage.segmentation import watershed -from squidpy._constants._constants import SegmentationBackend from squidpy._constants._pkg_constants import Key -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import NDArrayA, singledispatchmethod +from squidpy._validators import normalize_choice from squidpy.im._container import ImageContainer __all__ = ["SegmentationModel", "SegmentationWatershed", "SegmentationCustom"] @@ -267,12 +267,11 @@ def __str__(self) -> str: @d.dedent -@inject_docs(m=SegmentationBackend) def segment( img: ImageContainer, layer: str | None = None, library_id: str | Sequence[str] | None = None, - method: str | SegmentationModel | Callable[..., NDArrayA] = "watershed", + method: Literal["log", "dog", "doh", "watershed"] | SegmentationModel | Callable[..., NDArrayA] = "watershed", channel: int | None = 0, chunks: str | int | tuple[int, int] | None = None, lazy: bool = False, @@ -292,44 +291,48 @@ def segment( method Segmentation method to use. Valid options are: - - `{m.WATERSHED.s!r}` - :func:`skimage.segmentation.watershed`. + - `'watershed'` - :func:`skimage.segmentation.watershed`. %(custom_fn)s channel Channel index to use for segmentation. If `None`, use all channels. %(chunks_lazy)s - %(layer_added)s If `None`, use ``'segmented_{{model}}'``. + %(layer_added)s If `None`, use ``'segmented_{model}'``. thresh Threshold for creation of masked image. The areas to segment should be contained in this mask. If `None`, it is determined by `Otsu's method `_. - Only used if ``method = {m.WATERSHED.s!r}``. + Only used if ``method = 'watershed'``. geq Treat ``thresh`` as upper or lower bound for defining areas to segment. If ``geq = True``, mask is defined as ``mask = arr >= thresh``, meaning high values in ``arr`` denote areas to segment. - Only used if ``method = {m.WATERSHED.s!r}``. + Only used if ``method = 'watershed'``. %(copy_cont)s %(segment_kwargs)s Returns ------- - If ``copy = True``, returns a new container with the segmented image in ``'{{layer_added}}'``. + If ``copy = True``, returns a new container with the segmented image in ``'{layer_added}'``. Otherwise, modifies the ``img`` with the following key: - - :class:`squidpy.im.ImageContainer` ``['{{layer_added}}']`` - the segmented image. + - :class:`squidpy.im.ImageContainer` ``['{layer_added}']`` - the segmented image. """ layer = img._get_layer(layer) - kind = SegmentationBackend.CUSTOM if callable(method) else SegmentationBackend(method) + kind = ( + "custom" + if callable(method) + else normalize_choice(method, ("log", "dog", "doh", "watershed", "custom"), name="method") + ) layer_new = Key.img.segment(kind, layer_added=layer_added) kwargs["chunks"] = chunks library_id = img._get_library_ids(library_id) if not isinstance(method, SegmentationModel): - if kind == SegmentationBackend.WATERSHED: + if kind == "watershed": if channel is None and img[layer].shape[-1] > 1: raise ValueError("Watershed segmentation does not work with multiple channels.") method: SegmentationModel = SegmentationWatershed() # type: ignore[no-redef] - elif kind == SegmentationBackend.CUSTOM: + elif kind == "custom": if not callable(method): raise TypeError(f"Expected `method` to be a callable, found `{type(method)}`.") method = SegmentationCustom(func=method) diff --git a/tests/image/test_segmentation.py b/tests/image/test_segmentation.py index 4ef0c594a..da7509a58 100644 --- a/tests/image/test_segmentation.py +++ b/tests/image/test_segmentation.py @@ -7,7 +7,6 @@ import pytest from pytest_mock import MockerFixture -from squidpy._constants._constants import SegmentationBackend from squidpy._constants._pkg_constants import Key from squidpy.im import ( ImageContainer, @@ -93,7 +92,7 @@ def test_method(self, small_cont: ImageContainer, method: str | Callable): assert res.shape == small_cont.shape if callable(method): - method = SegmentationBackend.CUSTOM.s + method = "custom" assert Key.img.segment(method) in res From e849a93b2c263f383fa1b1a218d8c40514bd51e5 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:18:24 +0200 Subject: [PATCH 21/26] Convert spatial_scatter shape --- src/squidpy/_constants/_constants.py | 7 ------- src/squidpy/pl/_spatial.py | 3 +-- src/squidpy/pl/_spatial_utils.py | 23 +++++++++-------------- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index d75a9d9ad..f597b7e70 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -22,10 +22,3 @@ class InferDimensions(ModeEnum): DEFAULT = "default" CHANNELS_LAST = "channels_last" Z_LAST = "z_last" - - -@unique -class ScatterShape(ModeEnum): - CIRCLE = "circle" - SQUARE = "square" - HEX = "hex" diff --git a/src/squidpy/pl/_spatial.py b/src/squidpy/pl/_spatial.py index 6ea7a3854..f9db0fbaf 100644 --- a/src/squidpy/pl/_spatial.py +++ b/src/squidpy/pl/_spatial.py @@ -12,7 +12,6 @@ from matplotlib.figure import Figure from squidpy._compat import old_positionals -from squidpy._constants._constants import ScatterShape from squidpy._constants._pkg_constants import Key from squidpy._docs import d from squidpy.gr._utils import _assert_spatial_basis @@ -400,7 +399,7 @@ def _wrap_signature(wrapper: Callable[[Any], Any]) -> Callable[[Any], Any]: def spatial_scatter( adata: AnnData, *, - shape: _AvailShapes | None = ScatterShape.CIRCLE.v, + shape: _AvailShapes | None = "circle", **kwargs: Any, ) -> Axes | Sequence[Axes] | None: """ diff --git a/src/squidpy/pl/_spatial_utils.py b/src/squidpy/pl/_spatial_utils.py index 1a43b5451..ac540797b 100644 --- a/src/squidpy/pl/_spatial_utils.py +++ b/src/squidpy/pl/_spatial_utils.py @@ -6,7 +6,7 @@ from functools import partial from numbers import Number from types import MappingProxyType -from typing import TYPE_CHECKING, Any, Literal, NamedTuple +from typing import Any, Literal, NamedTuple, get_args import dask.array as da import numpy as np @@ -35,15 +35,15 @@ from skimage.util import map_array from squidpy._compat import add_categorical_legend, default_frameon, get_vector, vector_friendly -from squidpy._constants._constants import ScatterShape from squidpy._constants._pkg_constants import Key from squidpy._utils import NDArrayA -from squidpy._validators import assert_key_in_adata +from squidpy._validators import assert_key_in_adata, normalize_choice from squidpy.im._coords import CropCoords from squidpy.pl._color_utils import _get_palette, _maybe_set_colors from squidpy.pl._utils import _assert_value_in_obs type _AvailShapes = Literal["circle", "square", "hex"] +_SHAPES = get_args(_AvailShapes.__value__) # get_args() is empty on a PEP 695 alias itself type Palette_t = str | ListedColormap | None type _Normalize = Normalize | Sequence[Normalize] type _SeqStr = str | Sequence[str] @@ -503,7 +503,7 @@ def _shaped_scatter( s: float, c: NDArrayA | str, *, - shape: _AvailShapes | ScatterShape | None = ScatterShape.CIRCLE, + shape: _AvailShapes | None = "circle", norm: _Normalize | None = None, **kwargs: Any, ) -> PatchCollection: @@ -513,16 +513,13 @@ def _shaped_scatter( Adapted from `here `_. This code is under `The BSD 3-Clause License `_. """ - shape = ScatterShape(shape) - if TYPE_CHECKING: - assert isinstance(shape, ScatterShape) - shape = ScatterShape(shape) + shape = normalize_choice(shape, _SHAPES, name="shape") - if shape == ScatterShape.CIRCLE: + if shape == "circle": patches = [Circle((x, y), radius=s) for x, y, s in np.broadcast(x, y, s)] - elif shape == ScatterShape.SQUARE: + elif shape == "square": patches = [Rectangle((x - s, y - s), width=2 * s, height=2 * s) for x, y, s in np.broadcast(x, y, s)] - elif shape == ScatterShape.HEX: + elif shape == "hex": n = 6 r = s / (2 * np.sin(np.pi / n)) polys = np.stack([_make_poly(x, y, r, n, i) for i in range(n)], 1).swapaxes(0, 2) @@ -779,9 +776,7 @@ def _prepare_args_plot( raise ValueError(f"`use_raw={use_raw}` but AnnData object does not have raw.") # logic for image v. non-image data is handled here - shape = ScatterShape(shape) if shape is not None else shape # type: ignore - if TYPE_CHECKING: - assert isinstance(shape, ScatterShape) or shape is None + shape = normalize_choice(shape, _SHAPES, name="shape") if shape is not None else shape return ColorParams(shape, color, groups, alpha, img_alpha, use_raw) From 03e06502ff08f6cf7167d85ad6f56319896a0cce Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:20:35 +0200 Subject: [PATCH 22/26] Convert dims and fix the InferDims_t spellings --- src/squidpy/_constants/_constants.py | 5 ----- src/squidpy/im/_container.py | 26 +++++++++++--------------- src/squidpy/im/_io.py | 25 +++++++++++++------------ tests/image/test_io.py | 12 +++++------- 4 files changed, 29 insertions(+), 39 deletions(-) diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py index f597b7e70..d1421492a 100644 --- a/src/squidpy/_constants/_constants.py +++ b/src/squidpy/_constants/_constants.py @@ -17,8 +17,3 @@ class ImageFeature(ModeEnum): # _ligrec.py -@unique -class InferDimensions(ModeEnum): - DEFAULT = "default" - CHANNELS_LAST = "channels_last" - Z_LAST = "z_last" diff --git a/src/squidpy/im/_container.py b/src/squidpy/im/_container.py index 76f2ed82f..f36cf8f7e 100644 --- a/src/squidpy/im/_container.py +++ b/src/squidpy/im/_container.py @@ -23,15 +23,15 @@ from skimage.util import img_as_float from squidpy._compat import default_palette -from squidpy._constants._constants import InferDimensions from squidpy._constants._pkg_constants import Key -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import NDArrayA, singledispatchmethod from squidpy._validators import ( assert_in_range, assert_non_empty_sequence, assert_non_negative, assert_positive, + normalize_choice, ) from squidpy.gr._utils import _assert_spatial_basis from squidpy.im._coords import ( @@ -44,12 +44,11 @@ _update_attrs_scale, ) from squidpy.im._feature_mixin import FeatureMixin -from squidpy.im._io import _assert_dims_present, _infer_dimensions, _lazy_load_image +from squidpy.im._io import _INFER_DIMS, InferDims_t, _assert_dims_present, _infer_dimensions, _lazy_load_image type FoI_t = int | float type Pathlike_t = str | Path type Arraylike_t = NDArrayA | xr.DataArray -type InferDims_t = Literal["default", "prefer_channels", "prefer_z"] | Sequence[str] type Input_t = Pathlike_t | Arraylike_t | Literal["ImageContainer"] _ERROR_NOTIMPLEMENTED_LIBID = f"It seems there are multiple `library_id` in `adata.uns[{Key.uns.spatial!r}]`.\n \ Loading multiple images is not implemented (yet), please specify a `library_id`." @@ -224,12 +223,11 @@ def save(self, path: Pathlike_t, **kwargs: Any) -> None: @d.get_sections(base="add_img", sections=["Parameters", "Raises"]) @d.dedent - @inject_docs(id=InferDimensions) def add_img( self, img: Input_t, layer: str | None = None, - dims: InferDims_t = InferDimensions.DEFAULT.s, + dims: InferDims_t = "default", library_id: str | Sequence[str] | None = None, lazy: bool = True, chunks: str | tuple[int, ...] | None = None, @@ -248,9 +246,9 @@ def add_img( dims Where to save channel dimension when reading from a file or loading an array. Valid options are: - - `{id.CHANNELS_LAST.s!r}` - load the last non-spatial dimension as channels. - - `{id.Z_LAST.s!r}` - load the last non-spatial dimension as Z-dimension. - - `{id.DEFAULT.s!r}` - same as `{id.CHANNELS_LAST.s!r}`, but for 4-dimensional arrays, + - `'channels_last'` - load the last non-spatial dimension as channels. + - `'z_last'` - load the last non-spatial dimension as Z-dimension. + - `'default'` - same as `'channels_last'`, but for 4-dimensional arrays, tries to also load the first dimension as channels if the last non-spatial dimension is 1. - a sequence of dimension names matching the shape of ``img``, e.g. ``('y', 'x', 'z', 'channels')``. `'y'`, `'x'` and `'z'` must always be present. @@ -276,9 +274,7 @@ def add_img( If loading a specific data type has not been implemented. """ layer = self._get_next_image_id("image") if layer is None else layer - dims: InferDimensions | Sequence[str] = ( # type: ignore[no-redef] - InferDimensions(dims) if isinstance(dims, str) else dims - ) + dims = normalize_choice(dims, _INFER_DIMS, name="dims") if isinstance(dims, str) else dims res: xr.DataArray | None = self._load_img(img, chunks=chunks, layer=layer, copy=copy, dims=dims, **kwargs) if res is not None: @@ -327,7 +323,7 @@ def _( self, img_path: Pathlike_t, chunks: int | None = None, - dims: InferDimensions | tuple[str, ...] = InferDimensions.DEFAULT, + dims: InferDims_t = "default", **_: Any, ) -> xr.DataArray | None: def transform_metadata(data: xr.Dataset) -> xr.Dataset: @@ -373,7 +369,7 @@ def _( self, img: NDArrayA, copy: bool = True, - dims: InferDimensions | tuple[str, ...] = InferDimensions.DEFAULT, + dims: InferDims_t = "default", **_: Any, ) -> xr.DataArray: logg.debug(f"Loading `numpy.array` of shape `{img.shape}`") @@ -386,7 +382,7 @@ def _( img: xr.DataArray, copy: bool = True, warn: bool = True, - dims: InferDimensions | tuple[str, ...] = InferDimensions.DEFAULT, + dims: InferDims_t = "default", **_: Any, ) -> xr.DataArray: logg.debug(f"Loading `xarray.DataArray` of shape `{img.shape}`") diff --git a/src/squidpy/im/_io.py b/src/squidpy/im/_io.py index c3ce5bce1..ac99bebfd 100644 --- a/src/squidpy/im/_io.py +++ b/src/squidpy/im/_io.py @@ -1,7 +1,8 @@ from __future__ import annotations -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from pathlib import Path +from typing import Literal import dask.array as da import numpy as np @@ -12,10 +13,11 @@ from skimage.io import imread from tifffile import TiffFile -from squidpy._constants._constants import InferDimensions -from squidpy._docs import inject_docs from squidpy._utils import NDArrayA +type InferDims_t = Literal["default", "channels_last", "z_last"] | Sequence[str] +_INFER_DIMS = ("default", "channels_last", "z_last") + def _assert_dims_present(dims: tuple[str, ...], include_z: bool = True) -> None: missing_dims = ({"y", "x", "z"} if include_z else {"y", "x"}) - set(dims) @@ -97,10 +99,9 @@ def _get_image_shape_dtype(fname: str) -> tuple[tuple[int, ...], np.dtype[np.gen Image.MAX_IMAGE_PIXELS = old_max_image_pixels -@inject_docs(id=InferDimensions) def _infer_dimensions( obj: NDArrayA | xr.DataArray | str, - infer_dimensions: InferDimensions | tuple[str, ...] = InferDimensions.DEFAULT, + infer_dimensions: InferDims_t = "default", ) -> tuple[tuple[int, ...], tuple[str, ...], np.dtype[np.generic], tuple[int, ...]]: """ Infer dimension names of an array. @@ -112,9 +113,9 @@ def _infer_dimensions( infer_dimensions Policy that determines how to name the dimensions. Valid options are: - - `{id.CHANNELS_LAST.s!r}` - load `channels` dimension as `channels`. - - `{id.Z_LAST.s!r}` - load `z` dimension as `channels`. - - `{id.DEFAULT.s!r}` - only matters if the number of dimensions is `3` or `4`. + - `'channels_last'` - load `channels` dimension as `channels`. + - `'z_last'` - load `z` dimension as `channels`. + - `'default'` - only matters if the number of dimensions is `3` or `4`. If `z` dimension is `1`, load it as `z`. Otherwise, if `channels` dimension is `1`, load `z` dimension (now larger than `1`) as `channels`. Otherwise, load `z` dimension as `z` and `channels` as `channels`. @@ -147,7 +148,7 @@ def dims(order: list[int]) -> tuple[str, ...]: return tuple(np.array(["z", "y", "x", "channels"], dtype=object)[np.argsort(order)]) def infer(y: int, x: int, z: int, c: int) -> tuple[str, ...]: - if infer_dimensions == InferDimensions.DEFAULT: + if infer_dimensions == "default": if shape[z] == 1: return dims([z, y, x, c]) if shape[c] == 1: @@ -155,7 +156,7 @@ def infer(y: int, x: int, z: int, c: int) -> tuple[str, ...]: return dims([z, y, x, c]) - if infer_dimensions == InferDimensions.Z_LAST: + if infer_dimensions == "z_last": return dims([c, y, x, z]) return dims([z, y, x, c]) @@ -168,7 +169,7 @@ def infer(y: int, x: int, z: int, c: int) -> tuple[str, ...]: ndim = len(shape) - if not isinstance(infer_dimensions, InferDimensions): + if not isinstance(infer_dimensions, str): if ndim not in (2, 3, 4): raise ValueError(f"Expected the image to be either `2`, `3` or `4` dimensional, found `{ndim}`.") # explicitly passed dims as tuple @@ -214,7 +215,7 @@ def infer(y: int, x: int, z: int, c: int) -> tuple[str, ...]: def _lazy_load_image( fname: str | Path, - dims: InferDimensions | tuple[str, ...] = InferDimensions.DEFAULT, + dims: InferDims_t = "default", chunks: int | str | tuple[int, ...] | Mapping[str, int | str] | None = None, ) -> xr.DataArray: # TODO(michalk8): switch to pims for tiffs? or switch completely once lazy-loading of JPEGs is done diff --git a/tests/image/test_io.py b/tests/image/test_io.py index 8327f1894..c70422a8b 100644 --- a/tests/image/test_io.py +++ b/tests/image/test_io.py @@ -8,7 +8,6 @@ from pytest_mock import MockerFixture from skimage.io import imread -from squidpy._constants._constants import InferDimensions from squidpy.im._io import _get_image_shape_dtype, _infer_dimensions, _lazy_load_image @@ -49,13 +48,12 @@ def test_get_shape(self, shape: tuple[int, ...], tmpdir): np.testing.assert_array_equal(actual_shape, shape) assert actual_dtype == img.dtype, (actual_dtype, img.dtype) - @pytest.mark.parametrize("infer_dim", list(InferDimensions)) + @pytest.mark.parametrize("infer_dim", ["default", "channels_last", "z_last"]) @pytest.mark.parametrize( "shape", [(101, 64), (101, 64, 3), (3, 64, 101), (1, 101, 64, 3), (1, 101, 64, 1), (3, 101, 64, 1)] ) def test_infer_dimensions(self, shape: tuple[int, ...], infer_dim: str, mocker: MockerFixture): mocker.patch("squidpy.im._io._get_image_shape_dtype", return_value=(shape, np.uint8)) - infer_dim = InferDimensions(infer_dim) actual_shape, actual_dims, _, _ = _infer_dimensions("non_existent", infer_dim) if len(shape) == 2: @@ -63,26 +61,26 @@ def test_infer_dimensions(self, shape: tuple[int, ...], infer_dim: str, mocker: np.testing.assert_array_equal(actual_shape, shape + (1, 1)) elif len(shape) == 3: if shape[-1] == 3: - if infer_dim == InferDimensions.Z_LAST: + if infer_dim == "z_last": np.testing.assert_array_equal(actual_dims, ["channels", "y", "x", "z"]) else: np.testing.assert_array_equal(actual_dims, ["z", "y", "x", "channels"]) np.testing.assert_array_equal(actual_shape, (1,) + shape) else: - if infer_dim == InferDimensions.Z_LAST: + if infer_dim == "z_last": np.testing.assert_array_equal(actual_dims, ["z", "y", "x", "channels"]) else: np.testing.assert_array_equal(actual_dims, ["channels", "y", "x", "z"]) np.testing.assert_array_equal(actual_shape, shape + (1,)) elif len(shape) == 4: - if infer_dim == InferDimensions.DEFAULT: + if infer_dim == "default": if shape[0] == 1: np.testing.assert_array_equal(actual_dims, ["z", "y", "x", "channels"]) elif shape[-1] == 1: np.testing.assert_array_equal(actual_dims, ["channels", "y", "x", "z"]) else: np.testing.assert_array_equal(actual_dims, ["z", "y", "x", "channels"]) - elif infer_dim == InferDimensions.Z_LAST: + elif infer_dim == "z_last": np.testing.assert_array_equal(actual_dims, ["channels", "y", "x", "z"]) else: np.testing.assert_array_equal(actual_dims, ["z", "y", "x", "channels"]) From 2ce23f2dc49bc39d326fda03b519e13328884f1c Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:22:58 +0200 Subject: [PATCH 23/26] Declare introspected literals without `type` A PEP 695 `type` alias is lazily evaluated, so get_args() on it returns nothing, which is how spatial_scatter ended up normalizing against an empty option list. Aliases we read back at runtime are plain assignments instead, and the .__value__ unwrapping goes. --- src/squidpy/im/_io.py | 7 ++++--- src/squidpy/pl/_spatial_utils.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/squidpy/im/_io.py b/src/squidpy/im/_io.py index ac99bebfd..c2712b50d 100644 --- a/src/squidpy/im/_io.py +++ b/src/squidpy/im/_io.py @@ -2,7 +2,7 @@ from collections.abc import Mapping, Sequence from pathlib import Path -from typing import Literal +from typing import Literal, get_args import dask.array as da import numpy as np @@ -15,8 +15,9 @@ from squidpy._utils import NDArrayA -type InferDims_t = Literal["default", "channels_last", "z_last"] | Sequence[str] -_INFER_DIMS = ("default", "channels_last", "z_last") +_InferDim = Literal["default", "channels_last", "z_last"] +_INFER_DIMS = get_args(_InferDim) +InferDims_t = _InferDim | Sequence[str] def _assert_dims_present(dims: tuple[str, ...], include_z: bool = True) -> None: diff --git a/src/squidpy/pl/_spatial_utils.py b/src/squidpy/pl/_spatial_utils.py index ac540797b..dd3a30e3a 100644 --- a/src/squidpy/pl/_spatial_utils.py +++ b/src/squidpy/pl/_spatial_utils.py @@ -42,8 +42,8 @@ from squidpy.pl._color_utils import _get_palette, _maybe_set_colors from squidpy.pl._utils import _assert_value_in_obs -type _AvailShapes = Literal["circle", "square", "hex"] -_SHAPES = get_args(_AvailShapes.__value__) # get_args() is empty on a PEP 695 alias itself +_AvailShapes = Literal["circle", "square", "hex"] +_SHAPES = get_args(_AvailShapes) type Palette_t = str | ListedColormap | None type _Normalize = Normalize | Sequence[Normalize] type _SeqStr = str | Sequence[str] From 57f3841523f8393a1df2e2a5bc45d38526b047b8 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:32:56 +0200 Subject: [PATCH 24/26] Convert image features and delete the enums features was the last argument reaching an enum, so _constants/_constants.py and _constants/_utils.py both go: no more ModeEnum, PrettyEnum, .s or .v. _pkg_constants.Key stays. The Literal carries the enum's values, so 'histogram' rather than the COLOR_HIST member name that would have renamed the output column. --- src/squidpy/_constants/_constants.py | 19 ------------ src/squidpy/_constants/_utils.py | 35 ---------------------- src/squidpy/im/_feature.py | 45 ++++++++++++++-------------- tests/image/test_features.py | 7 ++--- 4 files changed, 26 insertions(+), 80 deletions(-) delete mode 100644 src/squidpy/_constants/_constants.py delete mode 100644 src/squidpy/_constants/_utils.py diff --git a/src/squidpy/_constants/_constants.py b/src/squidpy/_constants/_constants.py deleted file mode 100644 index d1421492a..000000000 --- a/src/squidpy/_constants/_constants.py +++ /dev/null @@ -1,19 +0,0 @@ -"""Constants that user deals with.""" - -from __future__ import annotations - -from enum import unique - -from squidpy._constants._utils import ModeEnum - - -@unique -class ImageFeature(ModeEnum): - TEXTURE = "texture" # doc: This would be a docstring. - SUMMARY = "summary" - COLOR_HIST = "histogram" - SEGMENTATION = "segmentation" - CUSTOM = "custom" - - -# _ligrec.py diff --git a/src/squidpy/_constants/_utils.py b/src/squidpy/_constants/_utils.py deleted file mode 100644 index 56e5415af..000000000 --- a/src/squidpy/_constants/_utils.py +++ /dev/null @@ -1,35 +0,0 @@ -from __future__ import annotations - -from enum import Enum -from typing import Any, NoReturn - - -class PrettyEnum(Enum): - """Enum with a pretty __str__ and __repr__.""" - - def __repr__(self) -> str: - return str(self) - - def __str__(self) -> str: - return str(self.value) - - -class ModeEnum(str, PrettyEnum): - """Enum which prints available values when invalid value has been passed.""" - - @classmethod - def _missing_(cls, value: object) -> NoReturn: - raise ValueError( - f"Invalid option `{value}` for `{cls.__name__}`. Valid options are: `{[m.value for m in cls]}`." - ) - - # TODO(michalk8): members now compare equal to their value, so `.s` is redundant at its 63 call sites. - @property - def s(self) -> str: - """Return the :attr:`value` as :class:`str`.""" - return str(self.value) - - @property - def v(self) -> Any: - """Alias for :attr:`value`.""" - return self.value diff --git a/src/squidpy/im/_feature.py b/src/squidpy/im/_feature.py index 53e0135e0..467482594 100644 --- a/src/squidpy/im/_feature.py +++ b/src/squidpy/im/_feature.py @@ -2,24 +2,26 @@ from collections.abc import Mapping, Sequence from types import MappingProxyType -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Literal, get_args import pandas as pd from anndata import AnnData from scanpy import logging as logg from squidpy._compat import old_positionals -from squidpy._constants._constants import ImageFeature -from squidpy._docs import d, inject_docs +from squidpy._docs import d from squidpy._utils import Signal, SigQueue, get_n_processes, parallelize +from squidpy._validators import normalize_choice from squidpy.gr._utils import _save_data from squidpy.im._container import ImageContainer +_Feature = Literal["texture", "summary", "histogram", "segmentation", "custom"] +_FEATURES = get_args(_Feature) + __all__ = ["calculate_image_features"] @d.dedent -@inject_docs(f=ImageFeature) @old_positionals( "img", "layer", @@ -38,7 +40,7 @@ def calculate_image_features( img: ImageContainer, layer: str | None = None, library_id: str | Sequence[str] | None = None, - features: str | Sequence[str] = ImageFeature.SUMMARY.s, + features: _Feature | Sequence[_Feature] = "summary", features_kwargs: Mapping[str, Mapping[str, Any]] = MappingProxyType({}), key_added: str = "img_features", copy: bool = False, @@ -59,20 +61,20 @@ def calculate_image_features( features Features to be calculated. Valid options are: - - `{f.TEXTURE.s!r}` - summary stats based on repeating patterns + - `'texture'` - summary stats based on repeating patterns :meth:`squidpy.im.ImageContainer.features_texture`. - - `{f.SUMMARY.s!r}` - summary stats of each image channel + - `'summary'` - summary stats of each image channel :meth:`squidpy.im.ImageContainer.features_summary`. - - `{f.COLOR_HIST.s!r}` - counts in bins of image channel's histogram + - `'histogram'` - counts in bins of image channel's histogram :meth:`squidpy.im.ImageContainer.features_histogram`. - - `{f.SEGMENTATION.s!r}` - stats of a cell segmentation mask + - `'segmentation'` - stats of a cell segmentation mask :meth:`squidpy.im.ImageContainer.features_segmentation`. - - `{f.CUSTOM.s!r}` - extract features using a custom function + - `'custom'` - extract features using a custom function :meth:`squidpy.im.ImageContainer.features_custom`. features_kwargs Keyword arguments for the different features that should be generated, such as - ``{{ {f.TEXTURE.s!r}: {{ ... }}, ... }}``. + ``{ 'texture': { ... }, ... }``. key_added Key in :attr:`anndata.AnnData.obsm` where to store the calculated features. %(copy)s @@ -86,7 +88,7 @@ def calculate_image_features( Otherwise, modifies the ``adata`` object with the following key: - - :attr:`anndata.AnnData.uns` ``['{{key_added}}']`` - the above mentioned dataframe. + - :attr:`anndata.AnnData.uns` ``['{key_added}']`` - the above mentioned dataframe. Raises ------ @@ -94,9 +96,9 @@ def calculate_image_features( If a feature is not known. """ layer = img._get_layer(layer) - if isinstance(features, str | ImageFeature): + if isinstance(features, str): features = [features] - features = sorted({ImageFeature(f).s for f in features}) + features = sorted({normalize_choice(f, _FEATURES, name="features") for f in features}) n_jobs = get_n_processes(n_jobs) start = logg.info(f"Calculating features `{list(features)}` using `{n_jobs}` core(s)") @@ -132,7 +134,7 @@ def _calculate_image_features_helper( img: ImageContainer, layer: str, library_id: str | Sequence[str] | None, - features: list[ImageFeature], + features: list[_Feature], features_kwargs: Mapping[str, Any], queue: SigQueue | None = None, **kwargs: Any, @@ -148,18 +150,17 @@ def _calculate_image_features_helper( features_dict = {} for feature in features: - feature = ImageFeature(feature) - feature_kwargs = features_kwargs.get(feature.s, {}) + feature_kwargs = features_kwargs.get(feature, {}) - if feature == ImageFeature.TEXTURE: + if feature == "texture": res = crop.features_texture(layer=layer, **feature_kwargs) - elif feature == ImageFeature.COLOR_HIST: + elif feature == "histogram": res = crop.features_histogram(layer=layer, **feature_kwargs) - elif feature == ImageFeature.SUMMARY: + elif feature == "summary": res = crop.features_summary(layer=layer, **feature_kwargs) - elif feature == ImageFeature.SEGMENTATION: + elif feature == "segmentation": res = crop.features_segmentation(intensity_layer=layer, **feature_kwargs) - elif feature == ImageFeature.CUSTOM: + elif feature == "custom": res = crop.features_custom(layer=layer, **feature_kwargs) else: # should never get here diff --git a/tests/image/test_features.py b/tests/image/test_features.py index a09564c84..3a1b2994b 100644 --- a/tests/image/test_features.py +++ b/tests/image/test_features.py @@ -8,7 +8,6 @@ from anndata import AnnData from pytest_mock import MockerFixture -from squidpy._constants._constants import ImageFeature from squidpy.im._container import ImageContainer from squidpy.im._feature import calculate_image_features @@ -171,7 +170,7 @@ def test_invalid_layer(self, adata: AnnData, cont: ImageContainer): calculate_image_features(adata, img=cont, layer="foo") def test_invalid_feature(self, adata: AnnData, cont: ImageContainer): - with pytest.raises(ValueError, match=r"Invalid option `foo` for `ImageFeature`"): + with pytest.raises(ValueError, match=r"Expected `features` to be one of"): calculate_image_features(adata, img=cont, features="foo") def test_passing_spot_crops_kwargs(self, adata: AnnData, cont: ImageContainer, mocker: MockerFixture): @@ -191,8 +190,8 @@ def dummy(_: np.ndarray, sentinel: bool = False) -> int: adata, img=cont, key_added="foo", - features=ImageFeature.CUSTOM.s, - features_kwargs={ImageFeature.CUSTOM.s: {"func": dummy, "sentinel": True, "channels": [0]}}, + features="custom", + features_kwargs={"custom": {"func": dummy, "sentinel": True, "channels": [0]}}, copy=True, ) From fcc9d41a3406d48c7c4dbe111b20bd056c04b74c Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 21:34:52 +0200 Subject: [PATCH 25/26] Drop the enum exception from the convention --- docs/contributing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.md b/docs/contributing.md index c8ec97dea..9c8bd62f4 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -171,7 +171,7 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors - **Keyword-only arguments**: a public function takes its data object (`adata`, `sdata` or `data`) positionally, and every other argument is keyword-only, e.g. `def nhood_enrichment(adata, *, cluster_key, ...)`. Avoid catch-all `**kwargs` that forward whatever they are given unchecked. - **Parameter bags**: a `*Params` {class}`~typing.TypedDict` in {mod}`squidpy.types`, each key declaring its default as `Annotated[type, Default(value)]`, only when the knobs depend on another argument (one bag per `method`, taken as `method_params`) or several functions share them (taken as `**kwargs: Unpack[XParams]`). Resolve it with `resolve_params` from `squidpy._params`, so unknown keys raise, and keep its keys out of the signature. -- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Case-sensitive: keys the user chose (obs/obsm keys, layer, channel or cluster names, paths), and arguments still typed as a `ModeEnum` from `squidpy._constants._constants` — convert those to a `Literal` rather than teaching the enum to fold case. +- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. From ba296998444a0c7dcbcfb391092f0d5f1d90bfe2 Mon Sep 17 00:00:00 2001 From: "selman.ozleyen" Date: Fri, 9 Oct 2026 22:05:27 +0200 Subject: [PATCH 26/26] Cover the converted arguments, derive option lists Add the eight missing rows to the casing table: shape, corr_axis, complex_policy, dendrogram, features, both method arguments and dims. They are what makes the contributing rule enforced rather than asserted, and shape is the one this branch already broke once. options_of leaves a single get_args call in the package and reads a lazy `type` alias too, so an option list cannot silently come back empty. _as_transform derives its tuple the same way instead of retyping it. _check_strategy words its own error rather than mirroring normalize_choice's, and the two spatial_neighbors parameter docs keep their original wording. --- docs/contributing.md | 2 +- src/squidpy/_validators.py | 10 ++- src/squidpy/experimental/im/_detect_tissue.py | 6 +- src/squidpy/experimental/im/_qc_image.py | 8 +-- .../experimental/tl/_stitched_labels.py | 3 +- src/squidpy/gr/_build.py | 15 ++++- src/squidpy/gr/_nhood.py | 6 +- src/squidpy/gr/neighbors.py | 8 ++- src/squidpy/im/_feature.py | 6 +- src/squidpy/im/_io.py | 5 +- src/squidpy/pl/_spatial_utils.py | 6 +- tests/experimental/test_stitched_labels.py | 4 +- tests/test_string_choices.py | 67 +++++++++++++++++++ 13 files changed, 116 insertions(+), 30 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 9c8bd62f4..cb4bc4e55 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -171,7 +171,7 @@ Consider enabling this option for [ruff][ruff-editors] and [biome][biome-editors - **Keyword-only arguments**: a public function takes its data object (`adata`, `sdata` or `data`) positionally, and every other argument is keyword-only, e.g. `def nhood_enrichment(adata, *, cluster_key, ...)`. Avoid catch-all `**kwargs` that forward whatever they are given unchecked. - **Parameter bags**: a `*Params` {class}`~typing.TypedDict` in {mod}`squidpy.types`, each key declaring its default as `Annotated[type, Default(value)]`, only when the knobs depend on another argument (one bag per `method`, taken as `method_params`) or several functions share them (taken as `**kwargs: Unpack[XParams]`). Resolve it with `resolve_params` from `squidpy._params`, so unknown keys raise, and keep its keys out of the signature. -- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. +- **String choices** (a `Literal[...]` such as `method="otsu"`) are case-insensitive. Resolve the argument once at the top of the public function with `value = normalize_choice(value, options, name=...)` from `squidpy._validators`, which returns the canonical spelling and raises naming the valid options. Keys the user chose (obs/obsm keys, layer, channel or cluster names, paths) stay case-sensitive. Derive a reused option set from its own annotation with `options_of` rather than retyping it: `_Feature = Literal[...]`, `_FEATURES = options_of(_Feature)`. - **Fits**, returned by one function for others to consume, are frozen dataclasses with methods (e.g. `.transform`). Pass them whole. - **Results**, returned instead of writing into the input, are {class}`~typing.NamedTuple`s. - **Randomness**: take `rng: SeedLike | RNGLike | None = None` ([SPEC 7](https://scientific-python.org/specs/spec-0007/)), never `seed`/`generator`/`random_state`; the exception is `random_state` on sklearn-style estimators (the clusterers), which follow sklearn's `get_params`/`set_params` protocol. Spawn one generator per independent task with `rng.spawn(n)`, and pass `legacy_random(rng)` to APIs that only take an integer seed. diff --git a/src/squidpy/_validators.py b/src/squidpy/_validators.py index e29439f6a..7d2cd8203 100644 --- a/src/squidpy/_validators.py +++ b/src/squidpy/_validators.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections.abc import Hashable, Iterable, Sequence -from typing import TYPE_CHECKING, Any, cast, overload +from typing import TYPE_CHECKING, Any, cast, get_args, overload from squidpy._utils import _unique_order_preserving @@ -90,6 +90,14 @@ def assert_isinstance(value: Any, expected_type: type | tuple[type, ...], *, nam raise TypeError(f"Expected `{name}` to be of type `{type_names}`, got `{type(value).__name__}`.") +def options_of(literal: Any) -> tuple[Any, ...]: + """Return the members of a `Literal`, so an option list is never retyped beside its annotation. + + Accepts a `type X = Literal[...]` alias too, whose lazy value `typing.get_args` alone would miss. + """ + return get_args(getattr(literal, "__value__", literal)) + + _RAISE = object() """Sentinel: `normalize_choice` raises rather than returning a miss.""" diff --git a/src/squidpy/experimental/im/_detect_tissue.py b/src/squidpy/experimental/im/_detect_tissue.py index 119163f9c..2fff21809 100644 --- a/src/squidpy/experimental/im/_detect_tissue.py +++ b/src/squidpy/experimental/im/_detect_tissue.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Iterator, Mapping, Sequence -from typing import Any, Literal, cast, get_args +from typing import Any, Literal, cast import dask.array as da import numpy as np @@ -27,7 +27,7 @@ _yx_from_shape, legacy_random, ) -from squidpy._validators import normalize_choice +from squidpy._validators import normalize_choice, options_of from squidpy.types import ( FelzenszwalbParams, WekaParams, @@ -261,7 +261,7 @@ def detect_tissue( Processing is performed at an appropriate resolution and then upscaled to match the original image dimensions. """ - method = normalize_choice(method, get_args(DetectTissueMethod), name="method") + method = normalize_choice(method, options_of(DetectTissueMethod), name="method") logger.info(f"Detecting tissue with method: {method}") diff --git a/src/squidpy/experimental/im/_qc_image.py b/src/squidpy/experimental/im/_qc_image.py index b4ca49585..ab414025a 100644 --- a/src/squidpy/experimental/im/_qc_image.py +++ b/src/squidpy/experimental/im/_qc_image.py @@ -2,7 +2,7 @@ from collections import defaultdict from collections.abc import Iterable -from typing import Literal, get_args +from typing import Literal import dask import dask.array as da @@ -16,7 +16,7 @@ from spatialdata.models import TableModel from squidpy._utils import _ensure_dim_order -from squidpy._validators import normalize_choice +from squidpy._validators import normalize_choice, options_of from squidpy.experimental.im._intensity_metrics import hed_metrics from squidpy.experimental.im._qc_metrics import _HNE_METRICS, InputKind, QCMetric, get_metric_info from squidpy.experimental.im._utils import ( @@ -131,9 +131,9 @@ def qc_image( else: metrics = list(metrics) - resolved = [normalize_choice(m, get_args(QCMetric), name="metrics", default=None) for m in metrics] + resolved = [normalize_choice(m, options_of(QCMetric), name="metrics", default=None) for m in metrics] if unknown := [m for m, match in zip(metrics, resolved, strict=True) if match is None]: - raise ValueError(f"Unknown metrics {unknown}. Available: {', '.join(get_args(QCMetric))}") + raise ValueError(f"Unknown metrics {unknown}. Available: {', '.join(options_of(QCMetric))}") metrics = resolved # Validate H&E constraint diff --git a/src/squidpy/experimental/tl/_stitched_labels.py b/src/squidpy/experimental/tl/_stitched_labels.py index 1317d1579..eb21d5bc1 100644 --- a/src/squidpy/experimental/tl/_stitched_labels.py +++ b/src/squidpy/experimental/tl/_stitched_labels.py @@ -194,8 +194,7 @@ def _check_strategy(strategy: str | Callable[[pd.Series], object]) -> str | Call match = normalize_choice(strategy, _BUILTIN_STRATEGIES, name="merge_strategy", default=None) if match is None: raise ValueError( - f"Expected `merge_strategy` to be one of `{list(_BUILTIN_STRATEGIES)}`, " - f"got `{strategy!r}`. Or pass a callable." + f"Expected `merge_strategy` to be a callable or one of `{list(_BUILTIN_STRATEGIES)}`, got `{strategy!r}`." ) return match diff --git a/src/squidpy/gr/_build.py b/src/squidpy/gr/_build.py index a33a256d7..741502543 100644 --- a/src/squidpy/gr/_build.py +++ b/src/squidpy/gr/_build.py @@ -193,8 +193,12 @@ def spatial_neighbors( `adata` is a :class:`spatialdata.SpatialData`. %(library_key)s coord_type - Type of coordinate system. If `None`, use `'grid'` when ``spatial_key`` is in - :attr:`anndata.AnnData.uns` with ``n_neighs = 6`` (Visium), otherwise `'generic'`. + Type of coordinate system. Valid options are: + + - `'grid'` - grid coordinates. + - `'generic'` - generic coordinates. + - `None` - `'grid'` if ``spatial_key`` is in :attr:`anndata.AnnData.uns` + with ``n_neighs = 6`` (Visium), otherwise use `'generic'`. n_neighs Depending on the ``coord_type``: @@ -217,7 +221,12 @@ def spatial_neighbors( percentile Percentile of the distances to use as threshold. Only used when ``coord_type = 'generic'``. transform - Adjacency matrix transform, or `None` for no transform. + Type of adjacency matrix transform. + Valid options are: + + - `'spectral'` - spectral transformation of the adjacency matrix. + - `'cosine'` - cosine transformation of the adjacency matrix. + - `None` - no transformation of the adjacency matrix. set_diag Whether to set the diagonal of the spatial connectivities to `1.0`. key_added diff --git a/src/squidpy/gr/_nhood.py b/src/squidpy/gr/_nhood.py index 05827ac44..e253dfe2c 100644 --- a/src/squidpy/gr/_nhood.py +++ b/src/squidpy/gr/_nhood.py @@ -4,7 +4,7 @@ import warnings from collections.abc import Sequence -from typing import Any, Literal, NamedTuple, get_args +from typing import Any, Literal, NamedTuple import numpy as np import pandas as pd @@ -33,7 +33,7 @@ get_n_numba_threads, numba_threads, ) -from squidpy._validators import assert_key_in_adata, assert_positive, normalize_choice +from squidpy._validators import assert_key_in_adata, assert_positive, normalize_choice, options_of from squidpy.gr._utils import ( _assert_categorical_obs, _assert_connectivity_key, @@ -45,7 +45,7 @@ __all__ = ["nhood_enrichment", "NhoodEnrichmentResult", "centrality_scores", "interaction_matrix"] CentralityScore = Literal["degree_centrality", "average_clustering", "closeness_centrality"] -_CENTRALITY_SCORES = get_args(CentralityScore) +_CENTRALITY_SCORES = options_of(CentralityScore) class NhoodEnrichmentResult(NamedTuple): diff --git a/src/squidpy/gr/neighbors.py b/src/squidpy/gr/neighbors.py index ab0932acf..7bf802695 100644 --- a/src/squidpy/gr/neighbors.py +++ b/src/squidpy/gr/neighbors.py @@ -27,7 +27,7 @@ from sklearn.neighbors import NearestNeighbors from squidpy._utils import NDArrayA -from squidpy._validators import assert_positive, normalize_choice +from squidpy._validators import assert_positive, normalize_choice, options_of from squidpy.gr._nhood import compute_hop_adjacency_matrices __all__ = [ @@ -44,13 +44,15 @@ "GridBuilder", ] -TransformLike = Literal["spectral", "cosine"] | None +_Transform = Literal["spectral", "cosine"] +TransformLike = _Transform | None """Adjacency-matrix transform, or `None` for no transform. Case-insensitive.""" +_TRANSFORMS = (*options_of(_Transform), None) def _as_transform(transform: TransformLike) -> TransformLike: """Resolve the ``transform`` argument to its canonical spelling, accepting any casing.""" - return normalize_choice(transform, ("spectral", "cosine", None), name="transform") + return normalize_choice(transform, _TRANSFORMS, name="transform") # Kept module-level (not folded into GraphBuilder's params): types the public diff --git a/src/squidpy/im/_feature.py b/src/squidpy/im/_feature.py index 467482594..d74ccf87a 100644 --- a/src/squidpy/im/_feature.py +++ b/src/squidpy/im/_feature.py @@ -2,7 +2,7 @@ from collections.abc import Mapping, Sequence from types import MappingProxyType -from typing import TYPE_CHECKING, Any, Literal, get_args +from typing import TYPE_CHECKING, Any, Literal import pandas as pd from anndata import AnnData @@ -11,12 +11,12 @@ from squidpy._compat import old_positionals from squidpy._docs import d from squidpy._utils import Signal, SigQueue, get_n_processes, parallelize -from squidpy._validators import normalize_choice +from squidpy._validators import normalize_choice, options_of from squidpy.gr._utils import _save_data from squidpy.im._container import ImageContainer _Feature = Literal["texture", "summary", "histogram", "segmentation", "custom"] -_FEATURES = get_args(_Feature) +_FEATURES = options_of(_Feature) __all__ = ["calculate_image_features"] diff --git a/src/squidpy/im/_io.py b/src/squidpy/im/_io.py index c2712b50d..0e1d5ee69 100644 --- a/src/squidpy/im/_io.py +++ b/src/squidpy/im/_io.py @@ -2,7 +2,7 @@ from collections.abc import Mapping, Sequence from pathlib import Path -from typing import Literal, get_args +from typing import Literal import dask.array as da import numpy as np @@ -14,9 +14,10 @@ from tifffile import TiffFile from squidpy._utils import NDArrayA +from squidpy._validators import options_of _InferDim = Literal["default", "channels_last", "z_last"] -_INFER_DIMS = get_args(_InferDim) +_INFER_DIMS = options_of(_InferDim) InferDims_t = _InferDim | Sequence[str] diff --git a/src/squidpy/pl/_spatial_utils.py b/src/squidpy/pl/_spatial_utils.py index dd3a30e3a..412948e6c 100644 --- a/src/squidpy/pl/_spatial_utils.py +++ b/src/squidpy/pl/_spatial_utils.py @@ -6,7 +6,7 @@ from functools import partial from numbers import Number from types import MappingProxyType -from typing import Any, Literal, NamedTuple, get_args +from typing import Any, Literal, NamedTuple import dask.array as da import numpy as np @@ -37,13 +37,13 @@ from squidpy._compat import add_categorical_legend, default_frameon, get_vector, vector_friendly from squidpy._constants._pkg_constants import Key from squidpy._utils import NDArrayA -from squidpy._validators import assert_key_in_adata, normalize_choice +from squidpy._validators import assert_key_in_adata, normalize_choice, options_of from squidpy.im._coords import CropCoords from squidpy.pl._color_utils import _get_palette, _maybe_set_colors from squidpy.pl._utils import _assert_value_in_obs _AvailShapes = Literal["circle", "square", "hex"] -_SHAPES = get_args(_AvailShapes) +_SHAPES = options_of(_AvailShapes) type Palette_t = str | ListedColormap | None type _Normalize = Normalize | Sequence[Normalize] type _SeqStr = str | Sequence[str] diff --git a/tests/experimental/test_stitched_labels.py b/tests/experimental/test_stitched_labels.py index ce890a2f3..aba57be4d 100644 --- a/tests/experimental/test_stitched_labels.py +++ b/tests/experimental/test_stitched_labels.py @@ -223,7 +223,7 @@ def test_aggregated_table_label_id_matches_new_element_ids(self, sdata_tile_boun ( "qc_and_stitch", {"labels_key": "labels", "merge_strategy": "bogus"}, - "Expected `merge_strategy` to be one of.*Or pass a callable", + "Expected `merge_strategy` to be a callable or one of", ), ( "qc_and_stitch", @@ -465,7 +465,7 @@ def test_invalid_merge_strategy_raises_without_table(self, sdata_tile_boundary): """M9: merge_strategy is validated eagerly even when write_table=False.""" sdata, _ = sdata_tile_boundary _qc_and_stitch(sdata) - with pytest.raises(ValueError, match="Expected `merge_strategy` to be one of.*Or pass a callable"): + with pytest.raises(ValueError, match="Expected `merge_strategy` to be a callable or one of"): sq.experimental.tl.make_stitched_labels( sdata, labels_key="labels", write_table=False, merge_strategy="bogus" ) diff --git a/tests/test_string_choices.py b/tests/test_string_choices.py index 5c8408248..d666e23b2 100644 --- a/tests/test_string_choices.py +++ b/tests/test_string_choices.py @@ -43,6 +43,35 @@ def _pl_ripley(adata, mode): sq.pl.ripley(adata, cluster_key="leiden", mode=mode) +def _ligrec(adata, **kw): + from itertools import product + + inter = tuple(product(adata.raw.var_names[:3], adata.raw.var_names[:3])) + return sq.gr.ligrec(adata, cluster_key="leiden", interactions=inter, n_perms=1, rng=0, copy=True, **kw) + + +def _pl_ligrec(adata, dendrogram): + res = _ligrec(adata) + sq.pl.ligrec(res, dendrogram=dendrogram, remove_empty_interactions=False) + + +def _im_features(_, features): + sq.im.calculate_image_features(_feature_adata(), _img(), features=features) + + +def _img(): + return sq.im.ImageContainer(np.random.default_rng(0).integers(0, 255, (50, 50, 3), dtype=np.uint8)) + + +def _feature_adata(): + import anndata as ad + + a = ad.AnnData(np.zeros((2, 2), dtype=np.float32)) + a.obsm["spatial"] = np.array([[10.0, 10.0], [30.0, 30.0]]) + a.uns["spatial"] = {"libid": {"scalefactors": {"spot_diameter_fullres": 10.0}}} + return a + + # (data factory name, call(data, value), upper-case spelling, error name in the message) _CASES = { "gr.nhood_enrichment-normalization": ( @@ -57,6 +86,44 @@ def _pl_ripley(adata, mode): "ZERO", "`handle_nan`", ), + "pl.spatial_scatter-shape": ( + "nhood_data", + lambda a, v: sq.pl.spatial_scatter(a, color="leiden", shape=v, library_id=None), + "SQUARE", + "`shape`", + ), + "gr.ligrec-corr_axis": ( + "adata", + lambda a, v: _ligrec(a, corr_axis=v, corr_method="fdr_bh"), + "CLUSTERS", + "`corr_axis`", + ), + "gr.ligrec-complex_policy": ( + "adata", + lambda a, v: _ligrec(a, complex_policy=v), + "ALL", + "`complex_policy`", + ), + "pl.ligrec-dendrogram": ("adata", _pl_ligrec, "BOTH", "`dendrogram`"), + "im.calculate_image_features-features": (None, _im_features, "HISTOGRAM", "`features`"), + "im.process-method": ( + None, + lambda _, v: sq.im.process(_img(), method=v), + "GRAY", + "`method`", + ), + "im.segment-method": ( + None, + lambda _, v: sq.im.segment(_img(), method=v), + "WATERSHED", + "`method`", + ), + "im.ImageContainer-dims": ( + None, + lambda _, v: sq.im.ImageContainer(np.zeros((4, 4, 3), dtype=np.uint8), dims=v), + "CHANNELS_LAST", + "`dims`", + ), "gr.centrality_scores-score": ( "nhood_data", lambda a, v: sq.gr.centrality_scores(a, cluster_key="leiden", score=v, copy=True),