Skip to content

Commit fafeac8

Browse files
CopilotLucaMarconatoclaude
authored
Align CLI arguments with reader APIs for full parameter consistency (#321)
* Initial plan * Fix CLI mismatches: add missing parameters for seqfish, visium, visium_hd, and macsima Co-authored-by: LucaMarconato <2664412+LucaMarconato@users.noreply.github.com> * Add complete CLI/API alignment: JSON kwargs parameters for all readers Co-authored-by: LucaMarconato <2664412+LucaMarconato@users.noreply.github.com> * 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 <noreply@anthropic.com> * add .gitignore * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * Replace unittest.mock with pytest-mock in CLI kwargs tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * add pytest mock --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: LucaMarconato <2664412+LucaMarconato@users.noreply.github.com> Co-authored-by: Luca Marconato <m.lucalmer@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 55c8539 commit fafeac8

5 files changed

Lines changed: 476 additions & 20 deletions

File tree

.gitignore

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ uv.lock
4545
profile.speedscope.json
4646

4747
# test datasets (e.g. Xenium ones)
48-
# symlinks
49-
data
5048
# data folder
5149
data/
5250
tests/data
5351
.venv
54-
.uv.lock

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ test = [
6161
"pytest",
6262
"pytest-cov",
6363
# https://github.com/scverse/spatialdata-io/issues/334
64-
"pyarrow!=22"
64+
"pyarrow!=22",
65+
"pytest-mock"
6566
]
6667
# this will be used by readthedocs and will make pip also look for pre-releases, generally installing the latest available version
6768
# update: readthedocs doens't seem to try to install pre-releases even if when trying to install the pre optional-dependency. For

0 commit comments

Comments
 (0)