refactor(subworkflows): extract params.* to take: inputs across 13 subworkflows - #152
Merged
heylf merged 11 commits intoMay 4, 2026
Merged
Conversation
Add inline comment documenting the semantics of each exit-code range in the errorStrategy retry list. Notably, 2147483647 is Integer.MAX_VALUE — Nextflow's sentinel for tasks that died before writing .exitcode (e.g. AWS Batch spot reclamation, kubernetes preemption, grid-scheduler cancellations) — not an AWS-specific exit code. Cite Nextflow docs/aws.md for the AWS case. Addresses PR #139 review comment r3165322845.
…m() for STARDIST ext.args
Replaces the Groovy-fallthrough `findAll()` (no closure) with the documented
stdlib `join` + `trim` chain. Internal double-spaces from empty-string entries
get collapsed by bash word-splitting at `${args}` interpolation; `.trim()`
removes leading/trailing whitespace.
Matches the existing project idiom used in
subworkflows/local/utils_nfcore_spatialxe_pipeline/main.nf:341,352
(toolCitationText / toolBibliographyText).
Addresses PR #139 review comment r3165345526.
This Dockerfile was unreferenced by any module: a repo-wide grep for "modules/local/utility/Dockerfile" returns zero hits. Each utility submodule (convert_mask_uint32, extract_dapi, resize_tif, segger2xr, etc.) declares its own container directly via `container "..."`. The associated `conda.yml` it expected (per the COPY instruction) does not exist next to it. Leftover from an earlier development phase; no longer needed. Addresses PR #139 review comment r3165438419.
…hema_input.json Add nf-schema `format` keys so the validator confirms paths exist: - bundle: format: directory-path - image: format: file-path These validate path existence at runtime, not name conventions — so the team's concern about varying 10x bundle/image naming doesn't conflict. The format key is documented at https://nextflow-io.github.io/nf-schema/latest/nextflow_schema/nextflow_schema_specification/ Addresses PR #139 review comments r3165291219 (C3) and r3165292878 (C4).
…ll via ext.args
Removes the inline `params.baysor_tiling_min_transcripts_per_cell` reference
from the stitch process script. Instead:
- The XENIUM_PATCH_STITCH withName: block in conf/modules.config sets the
flag via ext.args
- The .nf script declares `def args = task.ext.args ?: ''` and interpolates
`${args}` into the command
Per Florian's narrowing directive, only the cited param at line 40 is fixed.
Other params references in the file (if any) remain as-is for follow-up work.
Addresses PR #139 review comment r3165442902.
Replaces the three inline `python3 - <<'PYEOF'` heredoc blocks (parquet
column statistics, tile-split workaround, NaN bd.x fix) plus the bundle
symlink shell loop with a single module binary at
resources/usr/bin/run_create_dataset.py.
The .nf script block shrinks from ~190 lines to ~15. The container
(quay.io/dongzehe/segger:1.0.14) stays general — pipeline-specific
workarounds for upstream segger bugs now live in the module binary
where they can be reviewed, tested, and removed when fixed upstream.
Each WORKAROUND function in the binary has a section comment naming
the upstream bug and removal condition.
Pattern matches existing baysor reference modules
(modules/local/baysor/{create_dataset,preprocess}/) which use
resources/usr/bin/ scripts called without ${moduleDir}/ prefix.
Addresses PR #139 review comment r3165415041 (segger create_dataset only;
predict refactor in next commit; train kept as-is per single-line debug
P2 carve-out).
Replaces inline GPU enumeration (`python3 -c` for torch.cuda.device_count), predict_parquet.py path resolver, and two `sed -i` runtime patches against installed segger source with a single module binary at resources/usr/bin/run_predict.py. The .nf script block shrinks from ~50 lines to ~15. The two sed patches (torch.no_grad() for VRAM savings, deterministic GPU seed) move into a named `patch_predict_parquet()` function with a clear "remove once upstreamed to segger" comment. Container (quay.io/dongzehe/segger:1.0.14) stays general — patches live in the pipeline. Addresses PR #139 review comment r3165415041 (segger predict).
…: inputs Per the project rule that params.* should only be read in the entry workflow (main.nf), the SPATIALXE named workflow now declares formal `take:` inputs for all 23 unique params.* references that were inline in the workflow body. The 23 unique params (50 total reference sites in the workflow body): baysor_prior, baysor_tiling, buffer_samples, buffer_size, cellpose_model, features, gene_panel, gene_synonyms, method, mode, multiqc_config, multiqc_logo, multiqc_methods_description, offtarget_probe_tracking, outdir, probes_fasta, qupath_polygons, reference_annotations, relabel_genes, run_qc, segmentation_mask, tiling, xeniumranger_only Addresses PR #139 review comment r3165543921 thoroughly — full SPATIALXE-level sweep instead of just the cited buffer_samples / buffer_size, matching the project preference for clean workflow interfaces. Subworkflow-level params.* extractions (~82 references across local subworkflows) are out of scope and tracked separately.
… with tar.gz test data The format: directory-path / file-path validators added in d59fac3 break CI because: - bundle field accepts EITHER a directory path (real Xenium bundle) OR a .tar.gz archive URL (test data, extracted at runtime by UNTAR). Neither 'directory-path' nor 'file-path' fits both, and both fail against remote URLs. - image field is similar — can be a local OME-TIFF path, a URL, or empty. The reviewer's suggestion in r3165291219 / r3165292878 was reasonable in isolation but conflicts with this pipeline's dual-purpose input semantics where the same field can be a path or a URL or an archive. Reverts the format keys; keeps the type / pattern / errorMessage entries.
…cross 13 local subworkflows Extends PR #150's SPATIALXE-level params extraction down to all local subworkflows. After this commit, params.* is read only in the entry workflow (main.nf). workflows/spatialxe.nf and all subworkflows/local/*/main.nf declare formal take: blocks; named workflows and helper functions accept their inputs explicitly. 13 subworkflows refactored: utils_nfcore_spatialxe_pipeline (16 sites — validateInputParameters refactored to take 13 explicit args; PIPELINE_INITIALISATION take: block extended; main.nf passes them through) cellpose_baysor_import_segmentation (13) cellpose_resolift_morphology_ome_tif (9) xeniumranger_import_segmentation_redefine_bundle (8) baysor_run_transcripts_parquet (8) baysor_run_prior_segmentation_mask (6) baysor_generate_segfree (6) spatialdata_write_meta_merge (5) baysor_run_transcripts_parquet_tiled (5) stardist_resolift_morphology_ome_tif (2) segger_create_train_predict (2) xeniumranger_resegment_morphology_ome_tif (1) ficture_preprocess_model (1) SPATIALXE workflow's take: block extended from 24 to 40 inputs to cover all params consumed by the subworkflows it calls (alphabetical order). PIPELINE_INITIALISATION's take: block extended from 9 to 21 inputs to plumb validation params from main.nf. main.nf updated to pass the new params from the entry workflow. Closes the params-in-workflow anti-pattern across the whole pipeline. Verified with `nextflow inspect` (parses cleanly) and `nextflow run -profile test -stub-run` (pipeline assembles, parameter validation succeeds).
…dict The heredocs were inadvertently introduced when extracting the inline Python to module binaries. Project convention (CLAUDE.md "Version Reporting") is topic-channel only — no `versions.yml` files. Topic channels (versions_segger, versions_python, etc.) emit the actual versions; the hardcoded `segger: 0.1.0` heredoc was unused and risked going stale. No other modules/local/* writes a versions.yml file. Aligns the segger modules with the rest of the pipeline.
an-altosian
requested review from
heylf and
khersameesh24
and removed request for
heylf
April 30, 2026 21:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends PR #150's SPATIALXE-level params extraction down to all local subworkflows. After this PR,
params.*is read only in the entry workflow (main.nf) — bothworkflows/spatialxe.nfand everysubworkflows/local/*/main.nfdeclare formaltake:blocks. Honors the project rule "using params in workflow is not good" all the way down the call stack.Scope
13 subworkflows refactored (~67 reference sites collapsed into formal
take:interfaces):utils_nfcore_spatialxe_pipelinePIPELINE_INITIALISATION(9 → 21)cellpose_baysor_import_segmentationcellpose_resolift_morphology_ome_tifxeniumranger_import_segmentation_redefine_bundlebaysor_run_transcripts_parquetbaysor_run_prior_segmentation_maskbaysor_generate_segfreespatialdata_write_meta_mergebaysor_run_transcripts_parquet_tiledstardist_resolift_morphology_ome_tifsegger_create_train_predictxeniumranger_resegment_morphology_ome_tifficture_preprocess_modelPlus:
SPATIALXEworkflow'stake:block extended from 23 to 39 inputs (added 16 params used by subworkflows but not previously consumed at the SPATIALXE level), andmain.nfupdated to pass the new params from the entry workflow.Verification
grep -rE "params\.[a-zA-Z_]+" subworkflows/local/ workflows/spatialxe.nfreturns empty — zero references outsidemain.nf.nextflow config -profile testparses cleanly.nextflow inspect main.nf -profile testparses cleanly.nextflow run -profile test -stub-runvalidates input params successfully (one downstream stub error inSPATIALDATA_WRITE_RAW_BUNDLEforspatialdata.__version__is pre-existing on the parent branch — unrelated to this refactor).Notable implementation choices
take:blocks with the same order mirrored at every call site — argument-ordering swap risk is the highest danger in this kind of refactor; alphabetical ordering on both ends makes mismatches visible during review.cellpose_model/stardist_nuclei_modelas local file variables. Renamed those locals tocellpose_model_path/stardist_modelto avoid shadowing the newtake:inputs.utils_nfcore_spatialxe_pipelinewas refactored, not skipped. PlumbingvalidateInputParameters()through took 13 explicit parameters but kept the spec's stated end state ("onlymain.nfreads params").Depends on
Branched off
feature/spatialxe-params-take-inputsfor clean stacking. The diff againstnf-core:devwill narrow to just this PR's content once #148 and #150 land.Test plan
grepconfirms zeroparams.*outsidemain.nfnextflow configparsesnextflow inspectparsesnextflow run -stub-runvalidates input params successfully