From 7e9196c035a3f9e8a8cfdf4fe393185c040633c4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 14:44:35 +0000 Subject: [PATCH 01/10] Initial plan From 65e4f02097a6371b78c5f1d3cf4644360ebd9516 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 14:54:28 +0000 Subject: [PATCH 02/10] Fix CLI mismatches: add missing parameters for seqfish, visium, visium_hd, and macsima Co-authored-by: LucaMarconato <2664412+LucaMarconato@users.noreply.github.com> --- src/spatialdata_io/__main__.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/spatialdata_io/__main__.py b/src/spatialdata_io/__main__.py index c65e46f4..b5927723 100644 --- a/src/spatialdata_io/__main__.py +++ b/src/spatialdata_io/__main__.py @@ -262,6 +262,13 @@ def merscope_wrapper( default=None, help="Which sections to load. Provide one or more section indices. [default: All sections are loaded]", ) +@click.option( + "--raster-models-scale-factors", + type=float, + multiple=True, + default=None, + help="Scale factors for raster models. [default: None]", +) def seqfish_wrapper( input: str, output: str, @@ -271,9 +278,11 @@ def seqfish_wrapper( load_shapes: bool = True, cells_as_circles: bool = False, rois: list[int] | None = None, + raster_models_scale_factors: list[float] | None = None, ) -> None: """Seqfish conversion to SpatialData.""" rois = list(rois) if rois else None + raster_models_scale_factors = list(raster_models_scale_factors) if raster_models_scale_factors else None sdata = seqfish( # type: ignore[name-defined] # noqa: F821 input, load_images=load_images, @@ -282,6 +291,7 @@ def seqfish_wrapper( load_shapes=load_shapes, cells_as_circles=cells_as_circles, rois=rois, + raster_models_scale_factors=raster_models_scale_factors, ) sdata.write(output) @@ -351,6 +361,12 @@ def stereoseq_wrapper( default=None, help="Path to the scalefactors file. [default: None]", ) +@click.option( + "--var-names-make-unique", + type=bool, + default=True, + help="Whether to make variable names unique. [default: True]", +) def visium_wrapper( input: str, output: str, @@ -359,6 +375,7 @@ def visium_wrapper( fullres_image_file: str | Path | None = None, tissue_positions_file: str | Path | None = None, scalefactors_file: str | Path | None = None, + var_names_make_unique: bool = True, ) -> None: """Visium conversion to SpatialData.""" sdata = visium( # type: ignore[name-defined] # noqa: F821 @@ -368,6 +385,7 @@ def visium_wrapper( fullres_image_file=fullres_image_file, tissue_positions_file=tissue_positions_file, scalefactors_file=scalefactors_file, + var_names_make_unique=var_names_make_unique, ) sdata.write(output) @@ -412,6 +430,12 @@ def visium_wrapper( default=False, help="If true, annotates the table by labels. [default: False]", ) +@click.option( + "--var-names-make-unique", + type=bool, + default=True, + help="Whether to make variable names unique. [default: True]", +) def visium_hd_wrapper( input: str, output: str, @@ -422,6 +446,7 @@ def visium_hd_wrapper( fullres_image_file: str | Path | None = None, load_all_images: bool = False, annotate_table_by_labels: bool = False, + var_names_make_unique: bool = True, ) -> None: """Visium HD conversion to SpatialData.""" sdata = visium_hd( # type: ignore[name-defined] # noqa: F821 @@ -433,6 +458,7 @@ def visium_hd_wrapper( fullres_image_file=fullres_image_file, load_all_images=load_all_images, annotate_table_by_labels=annotate_table_by_labels, + var_names_make_unique=var_names_make_unique, ) sdata.write(output) @@ -502,6 +528,12 @@ def xenium_wrapper( @cli.command(name="macsima") @_input_output_click_options +@click.option( + "--parsing-style", + type=click.Choice(["auto", "processed_single_folder", "processed_multiple_folders", "raw"]), + default="auto", + help="Parsing style for MACSima data. [default: auto]", +) @click.option( "--filter-folder-names", type=str, @@ -568,6 +600,7 @@ def macsima_wrapper( input: str, output: str, *, + parsing_style: str = "auto", filter_folder_names: list[str] | None = None, subset: int | None = None, c_subset: int | None = None, @@ -585,6 +618,7 @@ def macsima_wrapper( """Read MACSima formatted dataset and convert to SpatialData.""" sdata = macsima( # type: ignore[name-defined] # noqa: F821 path=input, + parsing_style=parsing_style, filter_folder_names=filter_folder_names, subset=subset, c_subset=c_subset, From 232f0dffb80dd7bb4632702fb3f9f8e68e0ba341 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 14:59:00 +0000 Subject: [PATCH 03/10] Add complete CLI/API alignment: JSON kwargs parameters for all readers Co-authored-by: LucaMarconato <2664412+LucaMarconato@users.noreply.github.com> --- src/spatialdata_io/__main__.py | 329 ++++++++++++++++++++++++++++++++- 1 file changed, 320 insertions(+), 9 deletions(-) diff --git a/src/spatialdata_io/__main__.py b/src/spatialdata_io/__main__.py index b5927723..0507a05e 100644 --- a/src/spatialdata_io/__main__.py +++ b/src/spatialdata_io/__main__.py @@ -64,9 +64,27 @@ def _input_output_click_options(func: Callable[..., None]) -> Callable[..., None default=True, help="Whether the .fcs file is provided if False a .csv file is expected. [default: True]", ) -def codex_wrapper(input: str, output: str, fcs: bool = True) -> None: +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +def codex_wrapper( + input: str, + output: str, + fcs: bool = True, + imread_kwargs: str = "{}", +) -> None: """Codex conversion to SpatialData.""" - sdata = codex(input, fcs=fcs) # type: ignore[name-defined] # noqa: F821 + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + + sdata = codex(input, fcs=fcs, imread_kwargs=imread_kwargs_dict) # type: ignore[name-defined] # noqa: F821 sdata.write(output) @@ -74,9 +92,42 @@ def codex_wrapper(input: str, output: str, fcs: bool = True) -> None: @_input_output_click_options @click.option("--dataset-id", type=str, default=None, help="Name of the dataset [default: None]") @click.option("--transcripts", type=bool, default=True, help="Whether to load transcript information. [default: True]") -def cosmx_wrapper(input: str, output: str, dataset_id: str | None = None, transcripts: bool = True) -> None: +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) +def cosmx_wrapper( + input: str, + output: str, + dataset_id: str | None = None, + transcripts: bool = True, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", +) -> None: """Cosmic conversion to SpatialData.""" - sdata = cosmx(input, dataset_id=dataset_id, transcripts=transcripts) # type: ignore[name-defined] # noqa: F821 + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + + sdata = cosmx( # type: ignore[name-defined] # noqa: F821 + input, + dataset_id=dataset_id, + transcripts=transcripts, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + ) sdata.write(output) @@ -162,6 +213,24 @@ def dbit_wrapper( default=True, help="Whether to process the label image into a multiscale image [default: True]", ) +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) +@click.option( + "--labels-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Labels2DModel. [default: {}]", +) def iss_wrapper( input: str, output: str, @@ -172,8 +241,20 @@ def iss_wrapper( dataset_id: str = "region", multiscale_image: bool = True, multiscale_labels: bool = True, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", + labels_models_kwargs: str = "{}", ) -> None: """ISS conversion to SpatialData.""" + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + labels_models_kwargs_dict = json.loads(labels_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + sdata = iss( # type: ignore[name-defined] # noqa: F821 input, raw_relative_path, @@ -183,6 +264,9 @@ def iss_wrapper( dataset_id=dataset_id, multiscale_image=multiscale_image, multiscale_labels=multiscale_labels, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + labels_models_kwargs=labels_models_kwargs_dict, ) sdata.write(output) @@ -192,9 +276,47 @@ def iss_wrapper( "--input", "-i", type=click.Path(exists=True), help="Path to the mcmicro project directory.", required=True ) @click.option("--output", "-o", type=click.Path(), help="Path to the output.zarr file.", required=True) -def mcmicro_wrapper(input: str, output: str) -> None: +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) +@click.option( + "--labels-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Labels2DModel. [default: {}]", +) +def mcmicro_wrapper( + input: str, + output: str, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", + labels_models_kwargs: str = "{}", +) -> None: """Conversion of MCMicro to SpatialData.""" - sdata = mcmicro(input) # type: ignore[name-defined] # noqa: F821 + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + labels_models_kwargs_dict = json.loads(labels_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + + sdata = mcmicro( # type: ignore[name-defined] # noqa: F821 + input, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + labels_models_kwargs=labels_models_kwargs_dict, + ) sdata.write(output) @@ -219,6 +341,18 @@ def mcmicro_wrapper(input: str, output: str) -> None: @click.option("--cells-boundaries", type=bool, default=True, help="Whether to read cells boundaries. [default: True]") @click.option("--cells-table", type=bool, default=True, help="Whether to read cells table. [default: True]") @click.option("--mosaic-images", type=bool, default=True, help="Whether to read the mosaic images. [default: True]") +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) def merscope_wrapper( input: str, output: str, @@ -231,8 +365,18 @@ def merscope_wrapper( cells_boundaries: bool = True, cells_table: bool = True, mosaic_images: bool = True, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", ) -> None: """Merscope conversion to SpatialData.""" + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + sdata = merscope( # type: ignore[name-defined] # noqa: F821 input, vpt_outputs=vpt_outputs, @@ -244,6 +388,8 @@ def merscope_wrapper( cells_boundaries=cells_boundaries, cells_table=cells_table, mosaic_images=mosaic_images, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, ) sdata.write(output) @@ -269,6 +415,12 @@ def merscope_wrapper( default=None, help="Scale factors for raster models. [default: None]", ) +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) def seqfish_wrapper( input: str, output: str, @@ -279,8 +431,16 @@ def seqfish_wrapper( cells_as_circles: bool = False, rois: list[int] | None = None, raster_models_scale_factors: list[float] | None = None, + imread_kwargs: str = "{}", ) -> None: """Seqfish conversion to SpatialData.""" + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + rois = list(rois) if rois else None raster_models_scale_factors = list(raster_models_scale_factors) if raster_models_scale_factors else None sdata = seqfish( # type: ignore[name-defined] # noqa: F821 @@ -292,6 +452,7 @@ def seqfish_wrapper( cells_as_circles=cells_as_circles, rois=rois, raster_models_scale_factors=raster_models_scale_factors, + imread_kwargs=imread_kwargs_dict, ) sdata.write(output) @@ -304,9 +465,40 @@ def seqfish_wrapper( default="deepcell", help="What kind of labels to use. [default: 'deepcell']", ) -def steinbock_wrapper(input: str, output: str, labels_kind: Literal["deepcell", "ilastik"] = "deepcell") -> None: +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) +def steinbock_wrapper( + input: str, + output: str, + labels_kind: Literal["deepcell", "ilastik"] = "deepcell", + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", +) -> None: """Steinbock conversion to SpatialData.""" - sdata = steinbock(input, labels_kind=labels_kind) # type: ignore[name-defined] # noqa: F821 + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + + sdata = steinbock( # type: ignore[name-defined] # noqa: F821 + input, + labels_kind=labels_kind, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + ) sdata.write(output) @@ -322,15 +514,44 @@ def steinbock_wrapper(input: str, output: str, labels_kind: Literal["deepcell", @click.option( "--optional-tif", type=bool, default=False, help="If True, will read ``{xx.TISSUE_TIF!r}`` files. [default: False]" ) +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) def stereoseq_wrapper( input: str, output: str, dataset_id: str | None = None, read_square_bin: bool = True, optional_tif: bool = False, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", ) -> None: """Stereoseq conversion to SpatialData.""" - sdata = stereoseq(input, dataset_id=dataset_id, read_square_bin=read_square_bin, optional_tif=optional_tif) # type: ignore[name-defined] # noqa: F821 + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + + sdata = stereoseq( # type: ignore[name-defined] # noqa: F821 + input, + dataset_id=dataset_id, + read_square_bin=read_square_bin, + optional_tif=optional_tif, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + ) sdata.write(output) @@ -367,6 +588,18 @@ def stereoseq_wrapper( default=True, help="Whether to make variable names unique. [default: True]", ) +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) def visium_wrapper( input: str, output: str, @@ -376,8 +609,18 @@ def visium_wrapper( tissue_positions_file: str | Path | None = None, scalefactors_file: str | Path | None = None, var_names_make_unique: bool = True, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", ) -> None: """Visium conversion to SpatialData.""" + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + sdata = visium( # type: ignore[name-defined] # noqa: F821 input, dataset_id=dataset_id, @@ -386,6 +629,8 @@ def visium_wrapper( tissue_positions_file=tissue_positions_file, scalefactors_file=scalefactors_file, var_names_make_unique=var_names_make_unique, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, ) sdata.write(output) @@ -436,6 +681,24 @@ def visium_wrapper( default=True, help="Whether to make variable names unique. [default: True]", ) +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) +@click.option( + "--anndata-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to anndata. [default: {}]", +) def visium_hd_wrapper( input: str, output: str, @@ -447,8 +710,20 @@ def visium_hd_wrapper( load_all_images: bool = False, annotate_table_by_labels: bool = False, var_names_make_unique: bool = True, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", + anndata_kwargs: str = "{}", ) -> None: """Visium HD conversion to SpatialData.""" + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + anndata_kwargs_dict = json.loads(anndata_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + sdata = visium_hd( # type: ignore[name-defined] # noqa: F821 path=input, dataset_id=dataset_id, @@ -459,6 +734,9 @@ def visium_hd_wrapper( load_all_images=load_all_images, annotate_table_by_labels=annotate_table_by_labels, var_names_make_unique=var_names_make_unique, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + anndata_kwargs=anndata_kwargs_dict, ) sdata.write(output) @@ -492,6 +770,24 @@ def visium_hd_wrapper( help="Whether to read cells annotations in the AnnData table. [default: True]", ) @click.option("--n-jobs", type=int, default=1, help="Number of jobs. [default: 1]") +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) +@click.option( + "--image-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", +) +@click.option( + "--labels-models-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to Labels2DModel. [default: {}]", +) def xenium_wrapper( input: str, output: str, @@ -507,8 +803,20 @@ def xenium_wrapper( aligned_images: bool = True, cells_table: bool = True, n_jobs: int = 1, + imread_kwargs: str = "{}", + image_models_kwargs: str = "{}", + labels_models_kwargs: str = "{}", ) -> None: """Xenium conversion to SpatialData.""" + import json + + try: + imread_kwargs_dict = json.loads(imread_kwargs) + image_models_kwargs_dict = json.loads(image_models_kwargs) + labels_models_kwargs_dict = json.loads(labels_models_kwargs) + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") + sdata = xenium( # type: ignore[name-defined] # noqa: F821 input, cells_boundaries=cells_boundaries, @@ -522,6 +830,9 @@ def xenium_wrapper( aligned_images=aligned_images, cells_table=cells_table, n_jobs=n_jobs, + imread_kwargs=imread_kwargs_dict, + image_models_kwargs=image_models_kwargs_dict, + labels_models_kwargs=labels_models_kwargs_dict, ) sdata.write(output) From c48d4830f0766715ad8df5d0fa8303dff95aa7a3 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Mon, 4 May 2026 19:38:54 +0200 Subject: [PATCH 04/10] Refactor CLI kwargs handling and fix macsima imread_kwargs gap - Move import json to top-level and introduce _parse_json_param helper to eliminate repeated try/except blocks in every wrapper function - Add missing --imread-kwargs option to macsima_wrapper (the reader exposes it but the CLI did not) - Fix trailing whitespace in multi-line function signatures from the PR Co-Authored-By: Claude Sonnet 4.6 --- src/spatialdata_io/__main__.py | 189 ++++++++++----------------------- 1 file changed, 57 insertions(+), 132 deletions(-) diff --git a/src/spatialdata_io/__main__.py b/src/spatialdata_io/__main__.py index 0507a05e..19987dd1 100644 --- a/src/spatialdata_io/__main__.py +++ b/src/spatialdata_io/__main__.py @@ -1,4 +1,5 @@ import importlib +import json from collections.abc import Callable from pathlib import Path from typing import Any, Literal @@ -56,6 +57,14 @@ def _input_output_click_options(func: Callable[..., None]) -> Callable[..., None return func +def _parse_json_param(value: str, param_name: str) -> dict[str, Any]: + try: + result: dict[str, Any] = json.loads(value) + return result + except json.JSONDecodeError as e: + raise click.BadParameter(f"Invalid JSON for {param_name!r}: {e}") from e + + @cli.command(name="codex") @_input_output_click_options @click.option( @@ -71,20 +80,13 @@ def _input_output_click_options(func: Callable[..., None]) -> Callable[..., None help="JSON string of keyword arguments passed to imread. [default: {}]", ) def codex_wrapper( - input: str, - output: str, + input: str, + output: str, fcs: bool = True, imread_kwargs: str = "{}", ) -> None: """Codex conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - - sdata = codex(input, fcs=fcs, imread_kwargs=imread_kwargs_dict) # type: ignore[name-defined] # noqa: F821 + sdata = codex(input, fcs=fcs, imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs")) # type: ignore[name-defined] # noqa: F821 sdata.write(output) @@ -105,28 +107,20 @@ def codex_wrapper( help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", ) def cosmx_wrapper( - input: str, - output: str, - dataset_id: str | None = None, + input: str, + output: str, + dataset_id: str | None = None, transcripts: bool = True, imread_kwargs: str = "{}", image_models_kwargs: str = "{}", ) -> None: """Cosmic conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = cosmx( # type: ignore[name-defined] # noqa: F821 - input, - dataset_id=dataset_id, + input, + dataset_id=dataset_id, transcripts=transcripts, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), ) sdata.write(output) @@ -246,15 +240,6 @@ def iss_wrapper( labels_models_kwargs: str = "{}", ) -> None: """ISS conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - labels_models_kwargs_dict = json.loads(labels_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = iss( # type: ignore[name-defined] # noqa: F821 input, raw_relative_path, @@ -264,9 +249,9 @@ def iss_wrapper( dataset_id=dataset_id, multiscale_image=multiscale_image, multiscale_labels=multiscale_labels, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, - labels_models_kwargs=labels_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), + labels_models_kwargs=_parse_json_param(labels_models_kwargs, "labels_models_kwargs"), ) sdata.write(output) @@ -295,27 +280,18 @@ def iss_wrapper( help="JSON string of keyword arguments passed to Labels2DModel. [default: {}]", ) def mcmicro_wrapper( - input: str, + input: str, output: str, imread_kwargs: str = "{}", image_models_kwargs: str = "{}", labels_models_kwargs: str = "{}", ) -> None: """Conversion of MCMicro to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - labels_models_kwargs_dict = json.loads(labels_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = mcmicro( # type: ignore[name-defined] # noqa: F821 input, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, - labels_models_kwargs=labels_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), + labels_models_kwargs=_parse_json_param(labels_models_kwargs, "labels_models_kwargs"), ) sdata.write(output) @@ -369,14 +345,6 @@ def merscope_wrapper( image_models_kwargs: str = "{}", ) -> None: """Merscope conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = merscope( # type: ignore[name-defined] # noqa: F821 input, vpt_outputs=vpt_outputs, @@ -388,8 +356,8 @@ def merscope_wrapper( cells_boundaries=cells_boundaries, cells_table=cells_table, mosaic_images=mosaic_images, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), ) sdata.write(output) @@ -434,15 +402,6 @@ def seqfish_wrapper( imread_kwargs: str = "{}", ) -> None: """Seqfish conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - - rois = list(rois) if rois else None - raster_models_scale_factors = list(raster_models_scale_factors) if raster_models_scale_factors else None sdata = seqfish( # type: ignore[name-defined] # noqa: F821 input, load_images=load_images, @@ -450,9 +409,9 @@ def seqfish_wrapper( load_points=load_points, load_shapes=load_shapes, cells_as_circles=cells_as_circles, - rois=rois, - raster_models_scale_factors=raster_models_scale_factors, - imread_kwargs=imread_kwargs_dict, + rois=list(rois) if rois else None, + raster_models_scale_factors=list(raster_models_scale_factors) if raster_models_scale_factors else None, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), ) sdata.write(output) @@ -478,26 +437,18 @@ def seqfish_wrapper( help="JSON string of keyword arguments passed to Image2DModel. [default: {}]", ) def steinbock_wrapper( - input: str, - output: str, + input: str, + output: str, labels_kind: Literal["deepcell", "ilastik"] = "deepcell", imread_kwargs: str = "{}", image_models_kwargs: str = "{}", ) -> None: """Steinbock conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = steinbock( # type: ignore[name-defined] # noqa: F821 - input, + input, labels_kind=labels_kind, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), ) sdata.write(output) @@ -536,21 +487,13 @@ def stereoseq_wrapper( image_models_kwargs: str = "{}", ) -> None: """Stereoseq conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = stereoseq( # type: ignore[name-defined] # noqa: F821 - input, - dataset_id=dataset_id, - read_square_bin=read_square_bin, + input, + dataset_id=dataset_id, + read_square_bin=read_square_bin, optional_tif=optional_tif, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), ) sdata.write(output) @@ -613,14 +556,6 @@ def visium_wrapper( image_models_kwargs: str = "{}", ) -> None: """Visium conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = visium( # type: ignore[name-defined] # noqa: F821 input, dataset_id=dataset_id, @@ -629,8 +564,8 @@ def visium_wrapper( tissue_positions_file=tissue_positions_file, scalefactors_file=scalefactors_file, var_names_make_unique=var_names_make_unique, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), ) sdata.write(output) @@ -715,15 +650,6 @@ def visium_hd_wrapper( anndata_kwargs: str = "{}", ) -> None: """Visium HD conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - anndata_kwargs_dict = json.loads(anndata_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = visium_hd( # type: ignore[name-defined] # noqa: F821 path=input, dataset_id=dataset_id, @@ -734,9 +660,9 @@ def visium_hd_wrapper( load_all_images=load_all_images, annotate_table_by_labels=annotate_table_by_labels, var_names_make_unique=var_names_make_unique, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, - anndata_kwargs=anndata_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), + anndata_kwargs=_parse_json_param(anndata_kwargs, "anndata_kwargs"), ) sdata.write(output) @@ -808,15 +734,6 @@ def xenium_wrapper( labels_models_kwargs: str = "{}", ) -> None: """Xenium conversion to SpatialData.""" - import json - - try: - imread_kwargs_dict = json.loads(imread_kwargs) - image_models_kwargs_dict = json.loads(image_models_kwargs) - labels_models_kwargs_dict = json.loads(labels_models_kwargs) - except json.JSONDecodeError as e: - raise click.BadParameter(f"Invalid JSON in kwargs parameter: {e}") - sdata = xenium( # type: ignore[name-defined] # noqa: F821 input, cells_boundaries=cells_boundaries, @@ -830,9 +747,9 @@ def xenium_wrapper( aligned_images=aligned_images, cells_table=cells_table, n_jobs=n_jobs, - imread_kwargs=imread_kwargs_dict, - image_models_kwargs=image_models_kwargs_dict, - labels_models_kwargs=labels_models_kwargs_dict, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), + image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), + labels_models_kwargs=_parse_json_param(labels_models_kwargs, "labels_models_kwargs"), ) sdata.write(output) @@ -907,6 +824,12 @@ def xenium_wrapper( default=False, help="Whether to include the cycle number in the channel name. [default: False]", ) +@click.option( + "--imread-kwargs", + type=str, + default="{}", + help="JSON string of keyword arguments passed to imread. [default: {}]", +) def macsima_wrapper( input: str, output: str, @@ -925,12 +848,14 @@ def macsima_wrapper( split_threshold_nuclei_channel: int | None = 2, skip_rounds: list[int] | None = None, include_cycle_in_channel_name: bool = False, + imread_kwargs: str = "{}", ) -> None: """Read MACSima formatted dataset and convert to SpatialData.""" sdata = macsima( # type: ignore[name-defined] # noqa: F821 path=input, parsing_style=parsing_style, filter_folder_names=filter_folder_names, + imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), subset=subset, c_subset=c_subset, max_chunk_size=max_chunk_size, From 18c60dc05978d676a2db22b22c80bf8f184acda1 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Tue, 5 May 2026 09:57:32 +0200 Subject: [PATCH 05/10] add .gitignore --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 32771d1c..f6be9a84 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,7 @@ node_modules/ # test datasets (e.g. Xenium ones) data/ tests/data +uv.lock + +# benchmarks +.asv/ From e7e363204e6b71abbac6c5ff2e8386814a7b9a7a Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Tue, 5 May 2026 10:20:33 +0200 Subject: [PATCH 06/10] Fix CLI/reader mismatches after main merge and add JSON kwargs tests CLI fixes (all matching current reader APIs): - xenium: remove deprecated --n-jobs, fix cells_as_circles default False, add --gex-only (default True) - visium_hd: add missing type=bool to --load-segmentations-only, add --gex-only (default False) - seqfish: change --rois from IntRange to str (reader uses list[str]), change --raster-models-scale-factors from float to int Tests (test_xenium.py, no real data required): - test_cli_xenium_invalid_json_rejected: all three kwargs options reject malformed JSON with a clear error message - test_cli_xenium_valid_json_forwarded: valid JSON is parsed and forwarded to the reader as a dict (verified via mock) Co-Authored-By: Claude Sonnet 4.6 --- src/spatialdata_io/__main__.py | 32 +++++++++++++++----- tests/test_xenium.py | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/src/spatialdata_io/__main__.py b/src/spatialdata_io/__main__.py index 970edbbd..293260a8 100644 --- a/src/spatialdata_io/__main__.py +++ b/src/spatialdata_io/__main__.py @@ -366,14 +366,14 @@ def merscope_wrapper( @click.option("--cells-as-circles", type=bool, default=False, help="Whether to read cells as circles. [default: False]") @click.option( "--rois", - type=click.IntRange(min=0), + type=str, multiple=True, default=None, - help="Which sections to load. Provide one or more section indices. [default: All sections are loaded]", + help="Which sections to load. Provide one or more ROI identifiers. [default: All sections are loaded]", ) @click.option( "--raster-models-scale-factors", - type=float, + type=int, multiple=True, default=None, help="Scale factors for raster models. [default: None]", @@ -392,8 +392,8 @@ def seqfish_wrapper( load_points: bool = True, load_shapes: bool = True, cells_as_circles: bool = False, - rois: list[int] | None = None, - raster_models_scale_factors: list[float] | None = None, + rois: list[str] | None = None, + raster_models_scale_factors: list[int] | None = None, imread_kwargs: str = "{}", ) -> None: """Seqfish conversion to SpatialData.""" @@ -615,6 +615,7 @@ def visium_wrapper( ) @click.option( "--load-segmentations-only", + type=bool, default=None, help="If `True`, only the segmented cell boundaries and their associated counts will be loaded. All binned data will be skipped. [default: None, which will fall back to `False` with a deprecation warning]", ) @@ -630,6 +631,12 @@ def visium_wrapper( default=True, help="Whether to make variable names unique. [default: True]", ) +@click.option( + "--gex-only", + type=bool, + default=False, + help="If `True`, only load gene expression features. [default: False]", +) @click.option( "--imread-kwargs", type=str, @@ -661,6 +668,7 @@ def visium_hd_wrapper( load_all_images: bool = False, annotate_table_by_labels: bool = False, var_names_make_unique: bool = True, + gex_only: bool = False, imread_kwargs: str = "{}", image_models_kwargs: str = "{}", anndata_kwargs: str = "{}", @@ -680,6 +688,7 @@ def visium_hd_wrapper( load_all_images=load_all_images, annotate_table_by_labels=annotate_table_by_labels, var_names_make_unique=var_names_make_unique, + gex_only=gex_only, imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), anndata_kwargs=_parse_json_param(anndata_kwargs, "anndata_kwargs"), @@ -693,7 +702,7 @@ def visium_hd_wrapper( @click.option( "--nucleus-boundaries", type=bool, default=True, help="Whether to read Nucleus boundaries. [default: True]" ) -@click.option("--cells-as-circles", type=bool, default=None, help="Whether to read cells as circles. [default: None]") +@click.option("--cells-as-circles", type=bool, default=False, help="Whether to read cells as circles. [default: False]") @click.option("--cells-labels", type=bool, default=True, help="Whether to read cells labels (raster). [default: True]") @click.option( "--nucleus-labels", type=bool, default=True, help="Whether to read nucleus labels (raster). [default: True]" @@ -715,7 +724,12 @@ def visium_hd_wrapper( default=True, help="Whether to read cells annotations in the AnnData table. [default: True]", ) -@click.option("--n-jobs", type=int, default=1, help="Number of jobs. [default: 1]") +@click.option( + "--gex-only", + type=bool, + default=True, + help="If `True`, only load gene expression features. [default: True]", +) @click.option( "--imread-kwargs", type=str, @@ -740,7 +754,7 @@ def xenium_wrapper( *, cells_boundaries: bool = True, nucleus_boundaries: bool = True, - cells_as_circles: bool | None = None, + cells_as_circles: bool = False, cells_labels: bool = True, nucleus_labels: bool = True, transcripts: bool = True, @@ -748,6 +762,7 @@ def xenium_wrapper( morphology_focus: bool = True, aligned_images: bool = True, cells_table: bool = True, + gex_only: bool = True, imread_kwargs: str = "{}", image_models_kwargs: str = "{}", labels_models_kwargs: str = "{}", @@ -767,6 +782,7 @@ def xenium_wrapper( morphology_focus=morphology_focus, aligned_images=aligned_images, cells_table=cells_table, + gex_only=gex_only, imread_kwargs=_parse_json_param(imread_kwargs, "imread_kwargs"), image_models_kwargs=_parse_json_param(image_models_kwargs, "image_models_kwargs"), labels_models_kwargs=_parse_json_param(labels_models_kwargs, "labels_models_kwargs"), diff --git a/tests/test_xenium.py b/tests/test_xenium.py index a9beddaf..6c7ee33d 100644 --- a/tests/test_xenium.py +++ b/tests/test_xenium.py @@ -1,6 +1,7 @@ import math from pathlib import Path from tempfile import TemporaryDirectory +from unittest.mock import MagicMock, patch import numpy as np import pytest @@ -266,3 +267,55 @@ def test_xenium_other_feature_types(dataset: str, gex_only: bool) -> None: else: assert ValueError(f"Unexpected dataset {dataset}") + + +# ── CLI JSON kwargs tests (no real data needed) ─────────────────────────────── + + +@pytest.mark.parametrize( + "kwarg_name", + ["--imread-kwargs", "--image-models-kwargs", "--labels-models-kwargs"], +) +def test_cli_xenium_invalid_json_rejected(runner: CliRunner, tmp_path: Path, kwarg_name: str) -> None: + """Invalid JSON for any kwargs option must produce a non-zero exit and a clear error.""" + result = runner.invoke( + xenium_wrapper, + [ + "--input", + str(tmp_path), + "--output", + str(tmp_path / "out.zarr"), + kwarg_name, + "not-valid-json{", + ], + ) + assert result.exit_code != 0 + assert "Invalid JSON" in result.output + + +@pytest.mark.parametrize( + ("kwarg_name", "kwarg_param"), + [ + ("--imread-kwargs", "imread_kwargs"), + ("--image-models-kwargs", "image_models_kwargs"), + ("--labels-models-kwargs", "labels_models_kwargs"), + ], +) +def test_cli_xenium_valid_json_forwarded(runner: CliRunner, tmp_path: Path, kwarg_name: str, kwarg_param: str) -> None: + """Valid JSON kwargs must be parsed and forwarded to the xenium reader as a dict.""" + mock_sdata = MagicMock() + with patch("spatialdata_io.readers.xenium.xenium", return_value=mock_sdata) as mock_xenium: + result = runner.invoke( + xenium_wrapper, + [ + "--input", + str(tmp_path), + "--output", + str(tmp_path / "out.zarr"), + kwarg_name, + '{"chunks": 512}', + ], + ) + assert result.exit_code == 0, result.output + call_kwargs = mock_xenium.call_args.kwargs + assert call_kwargs[kwarg_param] == {"chunks": 512} From 7e5c71b1e41a915b75089ca0d729fc082f3bb191 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Tue, 5 May 2026 10:23:25 +0200 Subject: [PATCH 07/10] Add structural CLI/reader alignment test tests/test_cli_alignment.py checks that every non-path parameter of each reader function is exposed by its CLI wrapper. The test runs in CI on every PR so future reader additions automatically turn the suite red until __main__.py catches up. Design notes: - Uses inspect.signature on wrapper.callback (Click replaces the function with a Command object; the Python signature lives in .callback) - _READER_EXCEPTIONS allows intentionally hidden params (currently: xenium n_jobs, which is deprecated and has no effect) - No real data required; pure import + introspection Co-Authored-By: Claude Sonnet 4.6 --- tests/test_cli_alignment.py | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/test_cli_alignment.py diff --git a/tests/test_cli_alignment.py b/tests/test_cli_alignment.py new file mode 100644 index 00000000..efa77067 --- /dev/null +++ b/tests/test_cli_alignment.py @@ -0,0 +1,87 @@ +"""Structural test: CLI/reader parameter alignment. + +Every parameter of each reader function must be exposed by its CLI wrapper. +Runs automatically in CI on every PR to catch drift between readers and +__main__.py before it reaches users. + +When a reader gains a new parameter and this test turns red, fix it by adding a +corresponding @click.option and parameter to the relevant *_wrapper function in +__main__.py. If the parameter should intentionally stay hidden (e.g. deprecated), +add it to _READER_EXCEPTIONS with a comment explaining why. +""" + +import inspect +from typing import Any + +import pytest + +# (reader_module_path, reader_func_name, cli_wrapper_func_name) +_READERS = [ + ("spatialdata_io.readers.codex", "codex", "codex_wrapper"), + ("spatialdata_io.readers.cosmx", "cosmx", "cosmx_wrapper"), + ("spatialdata_io.readers.curio", "curio", "curio_wrapper"), + ("spatialdata_io.readers.dbit", "dbit", "dbit_wrapper"), + ("spatialdata_io.readers.iss", "iss", "iss_wrapper"), + ("spatialdata_io.readers.macsima", "macsima", "macsima_wrapper"), + ("spatialdata_io.readers.mcmicro", "mcmicro", "mcmicro_wrapper"), + ("spatialdata_io.readers.merscope", "merscope", "merscope_wrapper"), + ("spatialdata_io.readers.seqfish", "seqfish", "seqfish_wrapper"), + ("spatialdata_io.readers.steinbock", "steinbock", "steinbock_wrapper"), + ("spatialdata_io.readers.stereoseq", "stereoseq", "stereoseq_wrapper"), + ("spatialdata_io.readers.visium", "visium", "visium_wrapper"), + ("spatialdata_io.readers.visium_hd", "visium_hd", "visium_hd_wrapper"), + ("spatialdata_io.readers.xenium", "xenium", "xenium_wrapper"), +] + +# Parameters to skip in the reader (first positional path arg, and **kwargs catch-alls) +_READER_SKIP = {"path"} +# Parameters to skip in the wrapper (always present, not reader-specific) +_WRAPPER_SKIP = {"input", "output"} + +# Intentionally unexposed reader parameters: {reader_func_name: {param, ...}} +# Document *why* each entry exists so future contributors can re-evaluate. +_READER_EXCEPTIONS: dict[str, set[str]] = { + # n_jobs is deprecated in the xenium reader (kept for backward compat but + # has no effect); exposing it in the CLI would mislead users. + "xenium": {"n_jobs"}, +} + + +def _reader_params(module_path: str, func_name: str) -> set[str]: + import importlib + + module = importlib.import_module(module_path) + func = getattr(module, func_name) + sig = inspect.signature(func) + return { + name + for name, param in sig.parameters.items() + if param.kind not in (inspect.Parameter.VAR_KEYWORD, inspect.Parameter.VAR_POSITIONAL) + and name not in _READER_SKIP + } + + +def _wrapper_params(func_name: str) -> set[str]: + import importlib + + module = importlib.import_module("spatialdata_io.__main__") + obj = getattr(module, func_name) + # @cli.command replaces the function with a click.Command; the Python + # function lives in .callback. + func = obj.callback if hasattr(obj, "callback") else obj + sig = inspect.signature(func) + return {name for name in sig.parameters if name not in _WRAPPER_SKIP} + + +@pytest.mark.parametrize("module_path,reader_name,wrapper_name", _READERS) +def test_cli_exposes_all_reader_params(module_path: str, reader_name: str, wrapper_name: str) -> None: + """Every non-path reader parameter must appear in the CLI wrapper.""" + reader = _reader_params(module_path, reader_name) + wrapper = _wrapper_params(wrapper_name) + exceptions = _READER_EXCEPTIONS.get(reader_name, set()) + missing = reader - wrapper - exceptions + assert not missing, ( + f"{wrapper_name} is missing CLI parameters for reader '{reader_name}': {sorted(missing)}\n" + f"Add @click.option and a matching parameter to {wrapper_name} in __main__.py.\n" + f"If the parameter should intentionally be hidden, add it to _READER_EXCEPTIONS['{reader_name}']." + ) From c109ed8dbbc39a88d6b3778bb31bc2dfdf9f9981 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Tue, 5 May 2026 11:39:20 +0200 Subject: [PATCH 08/10] Replace unittest.mock with pytest-mock in CLI kwargs tests Co-Authored-By: Claude Sonnet 4.6 --- tests/test_xenium.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/test_xenium.py b/tests/test_xenium.py index 6c7ee33d..be353130 100644 --- a/tests/test_xenium.py +++ b/tests/test_xenium.py @@ -1,11 +1,11 @@ import math from pathlib import Path from tempfile import TemporaryDirectory -from unittest.mock import MagicMock, patch import numpy as np import pytest from click.testing import CliRunner +from pytest_mock import MockerFixture from spatialdata import match_table_to_element, read_zarr from spatialdata.models import TableModel, get_table_keys @@ -301,21 +301,23 @@ def test_cli_xenium_invalid_json_rejected(runner: CliRunner, tmp_path: Path, kwa ("--labels-models-kwargs", "labels_models_kwargs"), ], ) -def test_cli_xenium_valid_json_forwarded(runner: CliRunner, tmp_path: Path, kwarg_name: str, kwarg_param: str) -> None: +def test_cli_xenium_valid_json_forwarded( + runner: CliRunner, tmp_path: Path, mocker: MockerFixture, kwarg_name: str, kwarg_param: str +) -> None: """Valid JSON kwargs must be parsed and forwarded to the xenium reader as a dict.""" - mock_sdata = MagicMock() - with patch("spatialdata_io.readers.xenium.xenium", return_value=mock_sdata) as mock_xenium: - result = runner.invoke( - xenium_wrapper, - [ - "--input", - str(tmp_path), - "--output", - str(tmp_path / "out.zarr"), - kwarg_name, - '{"chunks": 512}', - ], - ) + mock_xenium = mocker.patch("spatialdata_io.readers.xenium.xenium") + mock_xenium.return_value = mocker.MagicMock() + result = runner.invoke( + xenium_wrapper, + [ + "--input", + str(tmp_path), + "--output", + str(tmp_path / "out.zarr"), + kwarg_name, + '{"chunks": 512}', + ], + ) assert result.exit_code == 0, result.output call_kwargs = mock_xenium.call_args.kwargs assert call_kwargs[kwarg_param] == {"chunks": 512} From 9f6e07a541461a9548af000e8a13d139f874fa26 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Tue, 5 May 2026 11:47:10 +0200 Subject: [PATCH 09/10] Improve test_cli_alignment: symmetric check and limitation docs - Check both directions (symmetric difference): reader params absent from CLI *and* CLI params absent from reader (stale options) - Expand docstring to explain what the test does NOT cover (types, defaults) and include a ready-to-run claude command for manual type/default audits Co-Authored-By: Claude Sonnet 4.6 --- tests/test_cli_alignment.py | 41 +++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/tests/test_cli_alignment.py b/tests/test_cli_alignment.py index efa77067..cab2c96a 100644 --- a/tests/test_cli_alignment.py +++ b/tests/test_cli_alignment.py @@ -1,13 +1,32 @@ """Structural test: CLI/reader parameter alignment. -Every parameter of each reader function must be exposed by its CLI wrapper. +Every parameter of each reader function must be exposed by its CLI wrapper, and +every wrapper parameter (beyond ``input``/``output``) must correspond to a reader +parameter. Both directions are checked as a symmetric difference. + Runs automatically in CI on every PR to catch drift between readers and __main__.py before it reaches users. -When a reader gains a new parameter and this test turns red, fix it by adding a -corresponding @click.option and parameter to the relevant *_wrapper function in -__main__.py. If the parameter should intentionally stay hidden (e.g. deprecated), -add it to _READER_EXCEPTIONS with a comment explaining why. +What this test does NOT check +------------------------------ +- **Types**: a reader may declare ``imread_kwargs: Mapping[str, Any]`` while the + CLI accepts a JSON string (``str``). The types will not match, and this test + will not catch that mismatch. +- **Default values**: a reader default and the corresponding ``@click.option`` + default can silently diverge (e.g. ``n_jobs=None`` vs ``default=1``). + +Both of these require manual or AI-assisted review. Run the following command +to ask Claude Code to audit type and default alignment for a specific reader:: + + claude "In src/spatialdata_io/__main__.py, compare every @click.option + default and type annotation in against the corresponding + parameter in the reader function in src/spatialdata_io/readers/.py. + List any mismatches in default values or types." + +When this test turns red, fix it by adding (or removing) the relevant +``@click.option`` and parameter in ``__main__.py``. If a reader parameter +should intentionally stay hidden (e.g. deprecated), add it to +``_READER_EXCEPTIONS`` with a comment explaining why. """ import inspect @@ -75,13 +94,23 @@ def _wrapper_params(func_name: str) -> set[str]: @pytest.mark.parametrize("module_path,reader_name,wrapper_name", _READERS) def test_cli_exposes_all_reader_params(module_path: str, reader_name: str, wrapper_name: str) -> None: - """Every non-path reader parameter must appear in the CLI wrapper.""" + """CLI wrapper and reader must have exactly the same parameter names (symmetric).""" reader = _reader_params(module_path, reader_name) wrapper = _wrapper_params(wrapper_name) exceptions = _READER_EXCEPTIONS.get(reader_name, set()) + + # Reader params absent from the CLI — the wrapper is missing options. missing = reader - wrapper - exceptions assert not missing, ( f"{wrapper_name} is missing CLI parameters for reader '{reader_name}': {sorted(missing)}\n" f"Add @click.option and a matching parameter to {wrapper_name} in __main__.py.\n" f"If the parameter should intentionally be hidden, add it to _READER_EXCEPTIONS['{reader_name}']." ) + + # Wrapper params absent from the reader — the CLI has stale/extra options. + extra = wrapper - reader - exceptions + assert not extra, ( + f"{wrapper_name} has CLI parameters not present in reader '{reader_name}': {sorted(extra)}\n" + f"Remove the corresponding @click.option from {wrapper_name} in __main__.py,\n" + f"or add the parameter to the reader if it was omitted by mistake." + ) From ce9e3b0b28ec1ddf8ffc9bb795e0f4c5437cd4d8 Mon Sep 17 00:00:00 2001 From: Luca Marconato Date: Tue, 5 May 2026 12:48:18 +0200 Subject: [PATCH 10/10] add pytest mock --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d97dc276..05c5baa2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,7 +61,8 @@ test = [ "pytest", "pytest-cov", # https://github.com/scverse/spatialdata-io/issues/334 - "pyarrow!=22" + "pyarrow!=22", + "pytest-mock" ] # this will be used by readthedocs and will make pip also look for pre-releases, generally installing the latest available version # update: readthedocs doens't seem to try to install pre-releases even if when trying to install the pre optional-dependency. For