Conversation
makes a new all_register target rule, and a do_seg global var, so segmentation is skipped (ie registration only enabled) when empty stains_for_seg or --no-segmentation is used. e.g. can now do --no-segmentation --register-to-mri to solely perform the registration part of the pipeline.
There was a problem hiding this comment.
Pull request overview
This PR adds a --no-segmentation CLI option to enable registration-only workflows in SPIMquant, while also upgrading the zarrnii dependency from version 0.14.1-alpha1 to 0.15.0. The changes introduce a new all_register target rule and use a global do_seg variable to conditionally skip segmentation steps when the option is enabled or when no segmentation stains are available.
Changes:
- Added
--no_segmentationCLI option to skip segmentation and quantification steps - Created new
all_registerrule to support registration-only workflows - Introduced
do_segvariable to control conditional execution of segmentation-related rules - Upgraded zarrnii dependency from 0.14.1-alpha1 to 0.15.0
- Refactored rule dependencies to separate registration outputs from segmentation outputs
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spimquant/workflow/Snakefile | Added do_seg variable logic, created all_register rule, modified all_segment rule to remove registration QC, added conditional includes for segmentation rules, updated all_participant and all_group rules |
| spimquant/config/snakebids.yml | Added --no_segmentation CLI argument definition with store_true action |
| pyproject.toml | Updated zarrnii version constraint from >=0.14.1-alpha1,<0.15.0 to >=0.15.0-alpha1,<0.16.0 |
| pixi.lock | Updated zarrnii package hash and version to 0.15.0, updated spimquant package hash |
| include: "rules/groupstats.smk" | ||
|
|
||
|
|
||
| if config["no_segmentation"] == False: |
There was a problem hiding this comment.
The conditional include uses only config["no_segmentation"] to decide whether to include segmentation rules, but the do_seg variable (lines 50-55) considers both empty stains_for_seg AND the no_segmentation flag. This creates an inconsistency: if stains_for_seg is empty but --no_segmentation is not set, the segmentation rules will be included but never executed (since do_seg would be False). Consider using if do_seg: instead of if config["no_segmentation"] == False: to ensure the conditional include logic matches the execution logic.
| if config["no_segmentation"] == False: | |
| if do_seg: |
| suffix="regqc.html", | ||
| **inputs["spim"].wildcards, | ||
| ), | ||
| stain=stain_for_reg, |
There was a problem hiding this comment.
Inconsistent list wrapping for the stain parameter. Line 188 wraps stain_for_reg in a list as stain=[stain_for_reg], but line 201 uses it directly as stain=stain_for_reg. Since stain_for_reg is a single string value (not a list), this inconsistency could cause issues. Check if the expand function expects a list at line 201 and adjust accordingly for consistency with line 188.
| stain=stain_for_reg, | |
| stain=[stain_for_reg], |
makes a new all_register target rule, and a do_seg global var, so segmentation is skipped (ie registration only enabled) when empty stains_for_seg or --no-segmentation is used.
e.g. can now do --no-segmentation --register-to-mri to solely perform the registration part of the pipeline.