Repository navigation
Conversation
1.0.1 hotfix
Switch fix-linting to the hardened nf-core/actions reusable workflow
Add per-stage QC collection modules that emit structured JSON, with a collector that produces MultiQC custom content YAML files for the final report. - New bin/emit_bundle_qc.py, emit_segmentation_qc.py, emit_pipeline_qc.py - New Nextflow modules: EMIT_BUNDLE_QC, EMIT_SEGMENTATION_QC, COLLECT_PIPELINE_QC - Wire QC collection into spatialaxe.nf for all mode/method combinations - Add segmentation_csv emit to cellpose_baysor, baysor_transcripts, and baysor_prior_mask subworkflows - Add section ordering to multiqc_config.yml - Add publishDir config for all QC modules in modules.config
This PR is against the
|
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 4.0.3. For more documentation on how to update your pipeline, please see the Synchronisation documentation. |
|
❌ nf-test failed with latest Nextflow versionNote Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
See the full run for details. |
Collaborator
|
@Jonahnki thank you for your contribution. Just make always sure to point towards the dev branch. I have already changed it. Just for the future. |
heylf
reviewed
Oct 16, 2026
|
|
||
| def compute_segmentation_metrics( | ||
| segmentation_csv: Path, | ||
| transcripts_csv: Path | None = None, |
Collaborator
There was a problem hiding this comment.
this is not used. Remove or check again.
heylf
reviewed
Oct 16, 2026
| ] | ||
| } | ||
|
|
||
| // ---------------------------- pipeline QC (Issue #208) ---------------------------------- |
Collaborator
There was a problem hiding this comment.
Suggested change
| // ---------------------------- pipeline QC (Issue #208) ---------------------------------- | |
| // ---------------------------- pipeline QC ---------------------------------- |
heylf
reviewed
Oct 16, 2026
|
|
||
| import argparse | ||
| import json | ||
| import sys |
Collaborator
There was a problem hiding this comment.
Suggested change
| import sys |
heylf
reviewed
Oct 16, 2026
|
|
||
| script: | ||
| """ | ||
| pip install --quiet pyyaml 2>/dev/null || true |
Collaborator
There was a problem hiding this comment.
Please dont do things like this. You have pyyaml anyway already covered in your container. If you need to install more packages then always put it into the docker. So please remove the line.
heylf
reviewed
Oct 16, 2026
Comment on lines
+23
to
+44
| mkdir -p qc_input mqc_output | ||
|
|
||
| # Stage all QC JSONs into a single directory | ||
| for f in ${qc_jsons}; do | ||
| cp "\$f" qc_input/ | ||
| done | ||
|
|
||
| # Also collect the merged records for downstream use | ||
| python3 -c " | ||
| import json, pathlib | ||
| records = [] | ||
| for p in sorted(pathlib.Path('qc_input').glob('*_qc.json')): | ||
| try: | ||
| data = json.loads(p.read_text()) | ||
| if isinstance(data, list): | ||
| records.extend(data) | ||
| else: | ||
| records.append(data) | ||
| except Exception: | ||
| pass | ||
| pathlib.Path('all_qc_records.json').write_text(json.dumps(records, indent=2)) | ||
| " |
Collaborator
There was a problem hiding this comment.
This can all be removed if you take the direct file paths which you get from path(qc_jsons). Instead of a wildcard you then go over all files of path(qc_jsons) in emit_pipeline_qc.py. What I mean is that path(qc_jsons) is a list of paths which you can provide as an arguemnt for emit_pipeline_qc.py directly.
So basically it would be better if emit_pipeline_qc.py handles the json agglomeration directly and that this step is not handled by some python code in the main.nf.
heylf
reviewed
Oct 16, 2026
heylf
left a comment
heylf
left a comment
Collaborator
There was a problem hiding this comment.
There are some requests to change a bit of code.
- Drop unused sys import and dead transcripts_csv param in emit_segmentation_qc.py (no caller ever passed it) - Simplify modules.config comment per review suggestion - Replace runtime pip install + inline python3 -c merge logic in COLLECT_PIPELINE_QC with a direct call to emit_pipeline_qc.py - emit_pipeline_qc.py: --qc-dir (glob) replaced with --qc-files (explicit file list); script now writes the merged output itself via --merged-output
Author
|
Thanks for the review — addressed all four points:
Verified with synthetic multi-sample/multi-stage QC JSON producing the correct merged output and all 4 MultiQC YAML sections, including a malformed-JSON edge case that skips gracefully. |
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
4 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
Adds per-stage QC collection modules that emit structured JSON at each pipeline stage, with a collector process that renders MultiQC custom content YAML files for the final report. Gives users cross-sample visibility into bundle metrics, segmentation quality and stage-level statistics without manual inspection.
What changed:
Architecture:
Each pipeline stage emits a self-contained QC JSON ({sample_id, stage, metrics}). The COLLECT_PIPELINE_QC process gathers all JSONs and produces MultiQC custom content YAML files that slot into the existing report. QC channel (ch_pipeline_qc_reports) is kept separate from upstream ch_qc_reports to avoid breaking existing OPT QC functionality.
MultiQC sections added:
Many thanks for the chance of contributing to nf-core/spatialaxe! Pls, let me know should there be any necessary PR changes on my path!