-
Notifications
You must be signed in to change notification settings - Fork 911
adding homer merge peaks, and a peakcalling workflow #9172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a comprehensive HOMER peak calling subworkflow with optional peak merging and annotation capabilities. The workflow builds on the existing HOMER modules to create a complete peak calling pipeline that can merge peaks across samples and annotate them with genomic features.
Key changes include:
- Added a new HOMER_MERGEPEAKS module for merging peak files across samples
- Created the HOMER_PEAKCALLING subworkflow that integrates multiple HOMER modules
- Updated HOMER version from 4.11 to 5.1 in the new components
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
modules/nf-core/homer/mergepeaks/main.nf | New module implementing peak merging functionality |
modules/nf-core/homer/mergepeaks/meta.yml | Module metadata for the mergepeaks module |
modules/nf-core/homer/mergepeaks/environment.yml | Conda environment specification with HOMER 5.1 |
modules/nf-core/homer/mergepeaks/tests/ | Test files for the mergepeaks module |
subworkflows/nf-core/homer_peakcalling/main.nf | Main subworkflow implementing complete peak calling pipeline |
subworkflows/nf-core/homer_peakcalling/meta.yml | Subworkflow metadata and documentation |
subworkflows/nf-core/homer_peakcalling/tests/ | Test files for the complete subworkflow |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
""" | ||
|
||
stub: | ||
def prefix = task.ext.prefix ?: "${meta.id}" |
Copilot
AI
Oct 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stub section uses a different default prefix pattern than the main script. The main script uses ${meta.id}_merged_peaks
while the stub uses just ${meta.id}
, which could cause inconsistent behavior between normal execution and stub mode.
def prefix = task.ext.prefix ?: "${meta.id}" | |
def prefix = task.ext.prefix ?: "${meta.id}_merged_peaks" |
Copilot uses AI. Check for mistakes.
annotate_individual // val: boolean - whether to annotate individual peak files | ||
|
||
main: | ||
if(!gtf && annotate_individual){ |
Copilot
AI
Oct 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Missing space after 'if' keyword. Should be if (!gtf && annotate_individual) {
to follow standard Groovy/Nextflow formatting conventions.
if(!gtf && annotate_individual){ | |
if (!gtf && annotate_individual) { |
Copilot uses AI. Check for mistakes.
ch_merged_peaks = HOMER_MERGEPEAKS.out.txt | ||
ch_versions = ch_versions.mix(HOMER_MERGEPEAKS.out.versions) | ||
|
||
if(gtf){ |
Copilot
AI
Oct 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Missing space after 'if' keyword. Should be if (gtf) {
to follow standard Groovy/Nextflow formatting conventions.
if(gtf){ | |
if (gtf) { |
Copilot uses AI. Check for mistakes.
…ut to homer/findpeaks. homer_peakcalling covers all styles
I needed a peak calling subworkflow for Homer that included annotation and MergePeaks.
This pull adds the homer MergePeaks module. It makes a couple modification to homer_groseq to include the MergePeaks step, and optional annotation.
Thought I'd offer it up -- @edmundmiller , if you're interested, this should function the same as homer_groseq if you pass in an empty channel for the gtf and set merge_peaks to
false
.Homer also has a major version update (currently most homer modules are 4.11, current is 5.1). If there is any interest in merging this into homer_groseq, then I'd be happy to update the other homer modules, also.