snakemake-wrappers icon indicating copy to clipboard operation
snakemake-wrappers copied to clipboard

feat: allow setting launchDir in params

Open tedil opened this issue 2 months ago • 7 comments

Because nextflow does not allow specifying the launchDir, but stores its .nextflow folder in that directory, the wrapper now allows changing into the directory specified in the param launch_dir before invoking nextflow. Not sure if this is the way to go / a robust way of doing so, though. Comments welcome.

Summary by CodeRabbit

  • New Features

    • Optional launch_dir support: runs can validate and switch to a specified working directory before execution to improve reproducibility.
  • Refactor

    • Parameter forwarding adjusted to exclude internal keys (including launch_dir) from downstream command inputs for cleaner behavior.
  • Documentation

    • Added configuration documentation describing the launch_dir option and its intended behavior.

tedil avatar Oct 09 '25 12:10 tedil

📝 Walkthrough

Walkthrough

Read launch_dir from Snakemake params and, if provided, validate and chdir into it before running the Nextflow shell command; replace multi-branch param filtering with a set-membership check excluding "launch_dir"; test Snakefile updated to name outputs and forward launch_dir; meta.yaml documents a new top-level params.launch_dir.

Changes

Cohort / File(s) Summary
Nextflow wrapper
utils/nextflow/wrapper.py
Replace multi-branch name filter with a set-membership check that excludes "launch_dir"; read launch_dir = params.get("launch_dir", None); if provided, validate existence and chdir into it before invoking the same shell Nextflow command.
Test Snakefile updates
utils/nextflow/test/Snakefile
Change rule chipseq_pipeline outputs: replace unnamed output "results/multiqc/broadPeak/multiqc_report.html" with multiqc_report="results/multiqc/broadPeak/multiqc_report.html" and add launch_dir=directory("some_directory"); add params.launch_dir=lambda wildcards, output: output.launch_dir; update outdir computation to use Path(output.multiqc_report).parents[-2].
Metadata
utils/nextflow/meta.yaml
Add top-level params section with a launch_dir field and description documenting that it lets users adjust the directory from which Nextflow is launched and noting Nextflow’s read-only launchDir behavior.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller
    participant Wrapper as Nextflow Wrapper
    participant FS as Filesystem
    participant Shell
    participant Nextflow

    Caller->>Wrapper: invoke wrapper(params)
    Wrapper->>Wrapper: filter params via set membership (exclude "launch_dir")
    Wrapper->>Wrapper: launch_dir = params.get("launch_dir", None)
    alt launch_dir provided
        Wrapper->>FS: stat/check launch_dir
        FS-->>Wrapper: exists / not exists
        alt exists
            Wrapper->>Wrapper: chdir(launch_dir)
        else not exists
            Wrapper-->>Caller: raise error
        end
    end
    Wrapper->>Shell: run nextflow shell command (from current dir)
    Shell->>Nextflow: execute
    Nextflow-->>Shell: result
    Shell-->>Wrapper: exit code/output
    Wrapper-->>Caller: return result

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description is a brief free-form note and does not follow the repository’s required template, as it lacks the QC checklist with tick boxes and omits confirmations for test updates, documentation links, and other mandatory sections. This makes it incomplete when compared to the provided description_template. Please expand the PR description to match the repository template by adding the QC section with the required checkboxes and filling in details on test.py updates, input/output path handling, documentation URLs in meta.yaml, and conda environment specifications.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main feature added—allowing the launchDir to be specified via parameters—and follows the conventional commit style prefix “feat:”. It accurately reflects the core change across the wrapper code, test Snakefile, and metadata without extraneous detail. It is clear and concise for anyone scanning the commit history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch utils/nextflow/allow-setting-launchdir

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df56b132597bb955d8bbe90cbf58ff998c5c5160 and d911057bdc0ce7e2a4b7ae082e8ead7349757a57.

📒 Files selected for processing (1)
  • utils/nextflow/meta.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: docs
  • GitHub Check: testing
  • GitHub Check: Summary
🔇 Additional comments (1)
utils/nextflow/meta.yaml (1)

9-12: Documentation clarifies the new launch_dir param

The added description cleanly explains the behavior and fits the wrapper metadata format. Nice!


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Oct 09 '25 12:10 coderabbitai[bot]

Do you think it would be possible to add a test case?

fgvieira avatar Oct 09 '25 12:10 fgvieira

I think I'd just modify the existing testcase to include this parameter, so we don't unnecessarily run the pipeline multiple times.

tedil avatar Oct 09 '25 13:10 tedil

And it would be great of you could also add it to the meta.yaml file. Just so people know what the param is supposed to do. :smile:

fgvieira avatar Oct 09 '25 13:10 fgvieira

Hm, I think the rabbit may potentially have a point regarding the relative-ness of paths; but the thing is that some nextflow pipelines resolve their (relative) paths relative to the launchDir… But I'd also like to avoid calling abspath or some other path normalization, to minimize issues with storage plugins.

tedil avatar Oct 09 '25 13:10 tedil

cc @famosab

johanneskoester avatar Oct 09 '25 13:10 johanneskoester

the thing is that some nextflow pipelines resolve their (relative) paths relative to the launchDir

I think all pipelines resolve that relative to the launchDir (except for inputs where you can give absolute paths) and you can define the results / work folders to be created somewhere else than the launchDir (both relative to the launch and with absolute paths). But the .nextflow/. files will be in the launchDir folder. Does that help at least a bit? I think setting the launchDir makes sense but it depends on what you want to achieve with the wrapper because I think the outdir can be set to a different path than the launchDir as well as the workdir.

famosab avatar Oct 10 '25 07:10 famosab