Skip to content

feat: migrate publishDir to workflow output definition (pilot, 15/19) - #65

Open
qclayssen wants to merge 2 commits into
refactor/nextflow-strict-syntaxfrom
feat/workflow-outputs
Open

feat: migrate publishDir to workflow output definition (pilot, 15/19)#65
qclayssen wants to merge 2 commits into
refactor/nextflow-strict-syntaxfrom
feat/workflow-outputs

Conversation

@qclayssen

Copy link
Copy Markdown
Collaborator

Summary

Pilots migrating sash's publishDir blocks to Nextflow's new workflow output definition (publish: section + top-level output {} block), which came out of preview and is stable as of Nextflow 25.10.0 — no feature flag needed on the 26.04.6 this branch already runs (built on top of #64). publishDir is being deprecated in favor of this.

Migrates 15 of sash's 19 publishDir blocks in conf/modules.config. Verified path-identical to the current publishDir/saveAs output layout via ~10 standalone empirical test pipelines, since the new syntax's directory/subdir-prefix semantics aren't well documented and differ subtly from legacy saveAs.

Base branch is refactor/nextflow-strict-syntax (#64) since this depends on the Nextflow 26.04.6 bump — rebase onto main once that merges.

What changed

  • conf/modules.config: removed publishDir from the 15 migrated withName: blocks.
  • main.nf: added the publish: section and a new top-level output {} block on the entry workflow {} (Nextflow only permits publish: in the unnamed entry workflow).
  • workflows/sash.nf / main.nf (UMCCR_SASH): added emit: blocks passing the 18 named channels needed by the publish paths up from the subworkflows to the entry workflow.
  • subworkflows/local/{purple_calling,linx_annotation,linx_plotting}.nf: added small additive _raw emits alongside each subworkflow's existing emit, exposing the process-scoped meta (needed for meta.key in publish paths). No logic changes.
  • nextflow.config: workflow.output.mode = params.publish_dir_mode, the new system's equivalent of the old per-block mode:.

What's deliberately NOT migrated (4 of 19)

BOLT_SMLV_SOMATIC_REPORT, BOLT_SMLV_GERMLINE_REPORT, BOLT_OTHER_CANCER_REPORT, BOLT_OTHER_MULTIQC_REPORT remain on publishDir in conf/modules.config, unchanged. All four do filename-pattern-based routing/renaming inside their saveAs closure — e.g. BOLT_SMLV_SOMATIC_REPORT pulls one specific *.pcgr.grch38.html out of a directory of PCGR output files and renames it to ${meta.key}/${meta.tumor_id}.pcgr.html, while everything else in that same process's output goes to a different subpath (${meta.key}/smlv_somatic/report/). BOLT_SMLV_GERMLINE_REPORT does the same for CPSR HTML; BOLT_OTHER_CANCER_REPORT/BOLT_OTHER_MULTIQC_REPORT do it for the cancer/multiqc reports.

The new workflow-output system's path {} closures operate on a channel's structured value (meta, file ->), not on arbitrary filenames matched against a directory the way saveAs does. Reproducing this exactly requires splitting each of these 4 processes' own output: block into separate named emit channels first (e.g. one channel for "the html report", one for "the rest of the output directory") — a change to the module .nf files themselves, not just config/workflow wiring. That's a materially bigger and riskier change than this pilot (touches process-level output declarations for report-generating processes whose output is what clinicians/curators actually review), so it's left as follow-up work, tracked here rather than attempted in this PR.

publishDir remains fully functional (deprecated, not removed) in the meantime, so nothing breaks by leaving these 4 as-is.

Verification

  • nextflow lint . -exclude .git -exclude work -exclude .nf-test -exclude nf-test.config -o concise — 0 errors, 0 warnings.
  • nextflow run . --help — parses cleanly.
  • NXF_SYNTAX_PARSER=v1 nf-test test — 11/13 passing, matching the known-good baseline on refactor: Nextflow 26.04.6 + strict syntax migration #64 exactly (2 pre-existing failures, unrelated).
  • nextflow run . -profile test,docker -stub-run — compiles the new publish:/output {} wiring with no structural error (fails only on the pre-existing conf/test.config missing-fixture gap tracked in refactor: Nextflow 26.04.6 + strict syntax migration #64, not on anything from this PR).
  • Not yet done: no real-data run to visually confirm the published directory tree matches today's — the empirical test pipelines proved the syntax patterns in isolation, but a real end-to-end run against a test sample (see docs/testing-validation.md tier 3) is the actual sign-off before this merges.

Follow-up (separate work, not this PR)

  • Split the 4 report-generating processes' output declarations to migrate the remaining publishDir blocks.
  • Real-data validation run comparing this branch's output tree to main's, byte-for-byte on the migrated paths.

@qclayssen

Copy link
Copy Markdown
Collaborator Author

Pushed `652c433` applying the confirmed simplification from the review: dropped the 4 redundant `_raw` emit channels (`purple_dir_raw`, `somatic_raw`, `germline_raw`, `plot_dir_raw`).

Cross-checked against Nextflow's own docs/tutorial for the workflow output system before making this change — there's no precedent there for a parallel "raw meta" channel; closures just read whatever field the channel's existing meta carries. Combined with the earlier empirical proof that `restoreMeta()`'s join means the restored channel's `meta.id` always equals what `meta.key` would be on the raw channel, this confirmed the simplification was safe: switched `main.nf`'s `purple_dir`/`linx_germline_annotation_dir`/`linx_somatic_annotation_dir`/`linx_somatic_plot_dir` path closures to read `meta.id` off the already-existing restored channels instead.

Also renamed `linxreport_raw` → `linxreport` for clarity — that one wasn't actually redundant (LINXREPORT's output never goes through `restoreMeta()`, so it was always the only channel available), just misleadingly named after the same pattern.

`nextflow lint`: 0/0. `nf-test`: 11/13 (unchanged). `--help` parses clean.

@qclayssen
qclayssen marked this pull request as ready for review August 5, 2026 04:45
@qclayssen
qclayssen force-pushed the feat/workflow-outputs branch from 652c433 to cf64145 Compare August 5, 2026 04:45
qclayssen added a commit that referenced this pull request Aug 7, 2026
run.sh's production invocation passes a bare --monochrome_logs flag
(no value), which Nextflow's CLI parser sets as the String "true" before
schema-based param coercion runs. Nextflow 26.04.6's stricter static
method dispatch no longer silently coerces that into
NfcoreTemplate.logColours(Boolean), so every #64/#65 run failed at
launch before submitting a single process.

Two independent crash sites, same root cause:
- NfcoreTemplate.logColours(): relaxed the Boolean-typed signature and
  coerce internally.
- NfcoreSchema.cleanParameters(): schema validation separately rejected
  the String "true" against the schema's declared boolean type; added a
  matching String->Boolean cast alongside the existing MemoryUnit/
  Duration/LinkedHashMap casts.

Verified locally under NXF_VER=26.04.6: --help and -stub-run with a bare
--monochrome_logs flag both clear params-summary and schema validation
with no MissingMethodException or ValidationException (previously
reproduced and fixed only as an unpushed, instance-local workaround on
i-05550e6eaf958e606, branch fix/64-monochrome-logs-coercion).
…(pilot)

Migrates the 15 "simple" publishDir/saveAs blocks (pure subdir+filename
formulas, no filename-content routing) to Nextflow's workflow output
definition (publish:/output{}), leaving the 4 filename-routing processes
(BOLT_SMLV_SOMATIC_REPORT, BOLT_SMLV_GERMLINE_REPORT,
BOLT_OTHER_CANCER_REPORT, BOLT_OTHER_MULTIQC_REPORT) on legacy publishDir.

Adds small pass-through `emit:` plumbing (process-scoped meta, needed for
`meta.key`) through purple_calling/linx_annotation/linx_plotting ->
workflow SASH -> UMCCR_SASH -> entry workflow, where the new publish:/
output{} blocks live. Path formulas were validated against live nextflow
runs (26.04.6) to byte-for-byte reproduce prior publishDir/saveAs output
layouts, including directory-flattening edge cases.
Code review + Nextflow docs research confirmed purple_dir_raw,
somatic_raw, germline_raw, and plot_dir_raw were unnecessary: each
subworkflow's already-restored emit (via WorkflowSash.restoreMeta())
carries a meta.id that's provably identical to what meta.key would be
on the raw channel, since restoreMeta joins ch_inputs on that same id.
Official Nextflow workflow-output docs/tutorial show no precedent for
a parallel "raw meta" channel pattern -- closures just read whatever
field the channel's existing meta already carries.

- purple_calling.nf / linx_annotation.nf: drop the _raw emits, keep
  only the restored ones.
- linx_plotting.nf: drop plot_dir_raw (redundant); rename
  linxreport_raw -> linxreport (not actually redundant -- LINXREPORT's
  output never goes through restoreMeta, so this was always the only
  channel, just misleadingly named after the same pattern).
- workflows/sash.nf: point SASH's emit block at the restored channels.
- main.nf: switch the corresponding output{} path closures from
  meta.key to meta.id to match.

nextflow lint: 0/0. nf-test: 11/13 (unchanged, 2 pre-existing failures
unrelated to this change). --help parses clean.
@qclayssen
qclayssen force-pushed the feat/workflow-outputs branch from cf64145 to 65f90a6 Compare August 7, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant