Conversation
- make bids spec with tpl in it - make alternate datatype folders so not everything is in micr - parc - seg - xfm (was warps) - tabular - vessels
There was a problem hiding this comment.
Pull request overview
This PR refactors SPIMquant’s derivative output layout to be more BIDS-derivatives-inspired by distributing outputs across multiple datatype folders (instead of concentrating them in micr/) and by introducing a template-level (tpl-) location for shared atlas LUTs/label tables. It also updates the SnakeBids spec and dependency to support a tpl-/template entity.
Changes:
- Update SnakeBids to v0.15 and extend the path spec to include a
templateentity (tagged astpl) for template-level outputs. - Reorganize many rule outputs into new datatypes (
parc,seg,xfm,tabular,featuremap,vessels) and removebids_tpl()in favor ofbids(..., template=...). - Centralize template LUT/label files (
dseg.tsv,*.itksnap.txt) under template-level (tpl-) paths rather than per-subject outputs.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
spimquant/workflow/Snakefile |
Bumps BIDS spec handling (adds template entity) and updates several rule all_* targets to new datatypes. |
spimquant/workflow/rules/common.smk |
Removes bids_tpl() and updates helpers to use bids(..., template=...); adjusts some tabular path helpers. |
spimquant/workflow/rules/import.smk |
Writes imported template assets (anat/spim/mask/dseg/tsv) to template-level bids(..., template=...) paths. |
spimquant/workflow/rules/templatereg.smk |
Renames warps → xfm, moves subject-warped atlas labels to parc/, and switches template asset references to template=. |
spimquant/workflow/rules/masking.smk |
Renames warps → xfm for init affine outputs. |
spimquant/workflow/rules/preproc_mri.smk |
Renames warps → xfm and updates MRI→SPIM QC report naming to from_/to_ entities. |
spimquant/workflow/rules/segmentation.smk |
Moves segmentation mask outputs (and related artifacts) into seg/ (including in work/). |
spimquant/workflow/rules/vessels.smk |
Moves vessel mask/SDT outputs into vessels/. |
spimquant/workflow/rules/fieldfrac.smk |
Generalizes fieldfrac paths to support `seg |
spimquant/workflow/rules/counts.smk |
Moves regionprops inputs to tabular/ and counts outputs to seg/. |
spimquant/workflow/rules/regionprops.smk |
Moves regionprops/coloc parquet outputs into tabular/; uses vessels/ for vessel SDT sampling. |
spimquant/workflow/rules/segstats.smk |
Moves atlas-based stats outputs into tabular/ and atlas dsegs into parc/; reads template LUTs via template= paths. |
spimquant/workflow/rules/heatmaps.smk |
Moves segstats TSV inputs to tabular/ and generated heatmaps to featuremap/; updates template LUT references. |
spimquant/workflow/rules/patches.smk |
Switches atlas dseg inputs to parc/ and mask-derived artifacts to seg/; reads LUTs via template= paths. |
spimquant/workflow/rules/groupstats.smk |
Updates group stats input collection to pull per-subject TSV/parquet from tabular/; updates template LUT references. |
pyproject.toml |
Updates snakebids constraint to >=0.15.0,<0.16. |
Comments suppressed due to low confidence (2)
spimquant/workflow/rules/counts.smk:23
counts.nii.gzis a derived scalar/count map rather than a segmentation mask. Writing it underdatatype="seg"is likely misleading; consider keeping count maps under a scalar-image datatype (e.g.,micr/) and reservingseg/for masks/segmentations.
counts_nii=bids(
root=root,
datatype="seg",
level="{level}",
stain="{stain}",
desc="{desc}",
suffix="counts.nii.gz",
spimquant/workflow/rules/counts.smk:56
counts.nii.gzin template space is a scalar/count map, not a segmentation. Consider using a scalar-image datatype (e.g.,micr/) instead ofseg/to keep datatypes semantically consistent.
counts_nii=bids(
root=root,
datatype="seg",
stain="{stain}",
space="{template}",
desc="{desc}",
suffix="counts.nii.gz",
| @@ -3,8 +3,15 @@ import os | |||
| from zarrnii import ZarrNii | |||
| from snakemake.utils import format | |||
| from snakebids import bids, generate_inputs, get_wildcard_constraints, set_bids_spec | |||
There was a problem hiding this comment.
get_wildcard_constraints and set_bids_spec are imported here but not used (the code calls snakebids.set_bids_spec directly). Removing unused imports (or using the imported set_bids_spec) will avoid confusion and keep the Snakefile tidy.
| from snakebids import bids, generate_inputs, get_wildcard_constraints, set_bids_spec | |
| from snakebids import bids, generate_inputs |
| report_html=bids( | ||
| root=root, | ||
| datatype="micr", | ||
| datatype="xfm", |
There was a problem hiding this comment.
This rule writes a QC HTML report (regqc.html) into the xfm/ datatype. Since xfm/ is otherwise used for transforms, consider moving QC artifacts to a dedicated datatype (e.g., qc/) to avoid mixing reports with transform files.
| datatype="xfm", | |
| datatype="qc", |
| root=root, | ||
| datatype="anat", | ||
| space="SPIM", | ||
| datatype="xfm", |
There was a problem hiding this comment.
This QC report (regqc.html) is being placed under datatype="xfm". To keep the output layout semantically clean, consider writing QC/visualization artifacts to a qc/ datatype instead of mixing them with transforms.
| datatype="xfm", | |
| datatype="qc", |
| counts_nii=bids( | ||
| root=root, | ||
| datatype="micr", | ||
| datatype="seg", | ||
| space="{template}", | ||
| desc="{desc}", | ||
| suffix="coloccounts.nii.gz", |
There was a problem hiding this comment.
coloccounts.nii.gz is a scalar count map, not a segmentation mask. Consider writing it under a scalar-image datatype (e.g., micr/) rather than seg/ to avoid mixing segmentations and scalar maps.
closes #136