Conversation
Important! Template update for nf-core/tools v3.3.2
Massive linting fixes and other minor changes
Added new module and subworkflow for opt, Ci fix changes
Diagnostic from previous commit confirmed the runner has only ONE disk (nvme0n1, 29G total) with NO secondary volume to relocate to. disk=large and disk=xlarge both map to the same 29G root. The runner image is runs-on-v2.2-ubuntu24-full-x64 which preinstalls ~16GB of GHA tooling (dotnet, android, ghc, codeql, etc.) that nf-test doesn't need. Swap to the slim noble variant: - image=ubuntu24-full-x64 (default; preinstalls full toolchain) + image=ubuntu24-noble-x64 (slim; only base ubuntu) The 29G volume now has ~28G free instead of ~12G — enough headroom for the cumulative pull (xeniumranger 7.92G + spatialdata 2G + proseg 0.8G ≈ 11G) within a single pipeline-mode stub test. Still no skip / no remove / no reduce / no switch.
@awgymer pointed at the runs-on `volume=` syntax. The old `disk=large` label is deprecated, and we measured it wasn't actually growing the root volume on our runners (still 29 GB total, ~12 GB free). Per runs-on docs migration guide: disk=large → volume=80gb https://runs-on.com/configuration/job-labels/#volume 80 GB gives ample headroom for the cumulative container pull on the image-mode pipeline stub test (~21 GB worst case: untar + spatialdata + proseg + cellpose + baysor + xeniumranger) on top of the runner's ~17 GB OS overhead.
CI passed with the volume=80gb label change suggested by @awgymer. Now reverting the other workflow-file experiments that were attempts to work around the underlying disk-pressure issue: - max_shards: 12 → 24 reverted to upstream/dev value (12) - "Inspect runner storage + free space" diagnostic+cleanup step removed - All earlier disk=large/disk=xlarge/image=noble experiments were already reverted Final delta on .github/workflows/nf-test.yml vs upstream/dev is now exactly one line: disk=large → volume=80gb.
…ation guard
## Motivation
Previously `validateXeniumBundle` ran during `PIPELINE_INITIALISATION` and
was gated by `if (!workflow.profile.contains('test'))`. The skip was needed
because:
* The test samplesheet points at a `.tar.gz` URL, not a directory; the
validation iterates required files like `cell_boundaries.csv.gz` over
that path and would always fail in test mode.
* For cloud-storage inputs (`s3://`, `gs://`, `az://`) the validation
also bailed out because `file().exists()` is unreliable before
Nextflow stages the input.
Both reasons describe the same root cause: validation was running BEFORE
staging, so it had to handle multiple un-staged input shapes.
## Change
Move validation to AFTER the `UNTAR` staging step, where the bundle is
unconditionally a resolved local directory (no matter whether the user
provided a directory, a remote tarball, or a cloud URL). This removes
the need for both the test-profile guard AND the cloud-storage early
return — the same validation runs uniformly for production and test.
* `subworkflows/local/utils_nfcore_spatialxe_pipeline/main.nf`:
delete the `validateXeniumBundle(ch_samplesheet)` call and its
`if (!workflow.profile.contains('test')) { ... }` guard. Delete the
`def validateXeniumBundle(ch_samplesheet)` function (it was tightly
coupled to the channel/un-staged-path shape).
* `workflows/spatialxe.nf`: inline the file-presence check into the
existing `ch_bundle_path` `.map` block. Required-file list errors;
optional-file list warns. Validation runs once per sample, after
`UNTAR` (test mode) or directly on the user-provided directory
(production).
## Side effect: UNTAR stub fix
Validating post-staging exposed an existing inconsistency between the
nf-core/untar real script and its stub: the real script uses
`tar --strip-components 1` when the archive has a single top-level
directory, but the stub iterates `tar -tf` output unchanged — which
places stubbed files OUTSIDE `${prefix}/` (at `./xenium_bundle/...`
in our case). The previous local untar patch worked around this by
adding 4 hardcoded `touch ${prefix}/...` lines for the files this
pipeline happens to need first.
The patch is replaced with a proper fix: in the stub's single-
top-level-dir branch, strip the leading directory from each tar
entry so the file layout matches what a real extraction produces.
The 4 hardcoded touches are removed because the loop now creates
them naturally. `modules/nf-core/untar/untar.diff` regenerated.
Snapshots regenerated to reflect the now-complete extracted-bundle
file listing (was previously truncated to the 4 hardcoded files).
## Verification
* `nf-test test tests/{default,coordinate_mode,image_mode,preview_mode,segfree_mode}.nf.test --profile=+docker --ci`
→ 5/5 PASS in ~450s after snapshot regen.
* Workflow runs to `workflow.success == true` against the test
samplesheet (`https://.../xenium_bundle.tar.gz`) — validation
now correctly sees the extracted directory contents.
* No new dependencies. The validation function definition was
deleted from utils_nfcore_spatialxe_pipeline/main.nf since the
inline check in spatialxe.nf is simpler than the old channel-based
iteration.
The legacy `disk=large` label is silently ignored by the current runs-on version — runners get 29 GB total / ~12 GB free, not enough for the cumulative container pull on heavy stub tests. `volume=80gb` (the new syntax) is honored and provisions 76 GB / ~60 GB free. This is the same one-line fix landed in PR #158 (commit 32f479d); PR #159 needs it independently since it branched from upstream/dev where the workflow file still has the old label.
Two reconciliations: 1. nf-test version drift. Local devs run nf-test 0.9.5; CI was pinned to 0.9.3. The two versions capture snapshot file metadata differently for gzip-format files (0.9.5 records full File-object shape, 0.9.3 records bare md5 strings), causing PR #159's locally-regenerated snapshots to mismatch CI's run output. Bumping CI to 0.9.5 (matching the published local default) makes the snapshots round-trip cleanly. 2. Pre-merge harmony with PR #158. PR #158 (params-in-modules cleanup) edits `subworkflows/local/utils_nfcore_spatialxe_pipeline/ main.nf` to add `format` plumbing and a method-aware format compatibility check. PR #159 (this PR) edits the same file to remove the `validateXeniumBundle` call + def. The two edits are in different parts of the file but to ensure PR #159 cleanly rebases onto PR #158 (which will merge first), the PR #158 additions are now present in PR #159's version of this file. No conflict on the eventual rebase.
Previous commit (8996511) tried to pre-emptively port PR #158's additions onto PR #159's subworkflow file. That broke things: PR #158's subworkflow additions include adding `format` as a take input, but PR #158's matching change to `main.nf` (passing `params.format` at the call site) was not pulled in (it's PR #158-only, not overlapping). Result: PIPELINE_INITIALISATION expected 22 take inputs but main.nf passed 21 → `workflow.success = false` in CI. Reverting the subworkflow file to PR #159's standalone version (removes validateXeniumBundle call + def, no other changes). When PR #158 merges first, the rebase of PR #159 will produce a normal 3-way merge on this file — git's auto-merge handles disjoint edits (PR #158's additions are in different line ranges from PR #159's removals), so no manual reconciliation is expected.
…apshots nf-test's FileUtil.getMd5() decompresses .gz files before hashing. When the UNTAR stub used `touch foo.gz` (0-byte file), GZIPInputStream threw EOF; the exception handler in PathConverter.java fell through to dumping raw File metadata (path, absolutePath, freeSpace, ...) into the JSON snapshot, causing machine-dependent dict-form entries. Fix: `: | gzip -n > file.gz` produces a 20-byte deterministic empty gzip (magic + empty deflate block, no embedded mtime/filename). Decompressed md5 is d41d8cd98f00b204e9800998ecf8427e on every machine, so snapshots are stable across local and CI runners. All 5 mode tests pass twice locally (regen + validate-without-update).
The original justifications for ignoring MULTIQC failures are gone: 1. `ext.args` title-flag bug (exit 2) — fixed; modules.config now emits `--title "..."` correctly for both PRE_XR and POST_XR variants. 2. `.map().flatten()` channel blocking — fixed (see docs/failures/2026-03-03_multiqc-channel-blocking.md). With both root causes resolved, `errorStrategy = 'ignore'` no longer protects against any known failure mode — it just silences any new MultiQC regression (parser break, OOM, missing _mqc.yml, etc.), letting the pipeline report success without a rendered report. Removing the directive restores a real CI signal for MultiQC failures.
…rams-strict fix(modules): address awgymer review comments on PR #139 (strict nf-core compliance)
…er-untar fix: validate Xenium bundle after UNTAR (drop test-profile skip-validation guard)
Collaborator
Author
|
@awgymer comments are solved. |
awgymer
requested changes
May 21, 2026
There was a problem hiding this comment.
Looking much better. Just one small change needed here.
Just to note that after this I am going to hold off on approval and green-lighting this release due to the pending decision on the potential re-scoping of this pipeline to support other spatial data as a name change of the pipeline would be more easily facilitated if there hasn't yet been a release. Hopefully a decision is made on that quickly.
Collaborator
Author
|
Yes, that it ok. We try to clarify as fast as possible. I agree that a potential name change should be done before the release. |
Collaborator
Author
|
Renaming of the pipeline is complete |
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
6 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.
The first release version of spatialxe. During the developemnt many tools were debugged, and new tools were added.
Since it is a first release thorough review is required.
PR checklist
nf-core lint).nextflow run . -profile test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).