Skip to content

Add fq2bam to methylseq#592

Closed
eduard-watchmaker wants to merge 11 commits into
devfrom
add_fq2bam_methylseq
Closed

Add fq2bam to methylseq#592
eduard-watchmaker wants to merge 11 commits into
devfrom
add_fq2bam_methylseq

Conversation

@eduard-watchmaker

Copy link
Copy Markdown
Contributor

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/methylseq branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

process {
withName: PARABRICKS_FQ2BAMMETH {
cache = 'lenient' // This is set because in the module command the index files are touched so as to have bwameth not complain
ext.args = '--low-memory'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this in all the runs, just on the tests

]
}

// SAMTOOLS_INDEX after PICARD_MARKDUPLICATES - publish to deduplicated when deduplication runs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this since we publish the *markdup.sorted.bam and .bai

@eduard-watchmaker

Copy link
Copy Markdown
Contributor Author

@nf-core-bot fix linting

@eduard-watchmaker

Copy link
Copy Markdown
Contributor Author

@nf-core-bot fix linting

@eduard-watchmaker

Copy link
Copy Markdown
Contributor Author

The linting really is giving me a hard time lol

@eduard-watchmaker

Copy link
Copy Markdown
Contributor Author

Tests 2 for conda, singularity and docker show different snapshots for the 'latest' version vs the 25.04, unsure why.

@nf-core-bot

Copy link
Copy Markdown
Member

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.5.1.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

@FelixKrueger
FelixKrueger enabled auto-merge July 19, 2026 16:51
@FelixKrueger

Copy link
Copy Markdown
Contributor

Thanks a lot for this, @eduard-watchmaker! 🙏

Since this PR was opened, parabricks/fq2bam has already landed on dev: the module is installed under modules/nf-core/parabricks/fq2bam/ and wired into the fastq_align_dedup_bwamem subworkflow with the same design this PR uses — a use_gpu gate, output_fmt, known_sites, and ch_alignment = PARABRICKS_FQ2BAM.out.bam. It came in via #610 (Update Bismark to 3.0.0 and modernise alignment modules/subworkflows), at a newer module pin (f0b0dc3… vs this PR's 1e4345f…).

That makes this PR superseded — its core purpose (adding fq2bam to the bwa-mem path) is already on dev, and it's ~37 commits behind with conflicts on the fq2bam module files themselves. So I'm closing it to keep the queue tidy.

The only thing here not on dev is a small conf/modules/parabricks_fq2bam.config; dev's fq2bam runs fine without it, but we can add process-level config as a quick standalone follow-up if it turns out to be useful.

Really appreciate the contribution — the approach matched exactly where dev landed. 🙌

@FelixKrueger
FelixKrueger disabled auto-merge July 20, 2026 22: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.

4 participants