Skip to content
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

Improved ext.args consolidation for STAR and TRIMGALORE #1248

Merged
merged 5 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Thank you to everyone else that has contributed by reporting bugs, enhancements
- [PR #1245](https://github.com/nf-core/rnaseq/pull/1245) - nf test quantify rsem
- [PR #1246](https://github.com/nf-core/rnaseq/pull/1246) - nf-test quantify pseudoalignment
- [PR #1247](https://github.com/nf-core/rnaseq/pull/1247) - nf-test prepare_genome
- [PR #1248](https://github.com/nf-core/rnaseq/pull/1248) - Improved ext.args consolidation for STAR and TRIMGALORE extra_args parameters
- [PR #1249](https://github.com/nf-core/rnaseq/pull/1249) - Include nf-tests for rsem_merge_counts module
- [PR #1250](https://github.com/nf-core/rnaseq/pull/1250) - Remove all tags.yml files because the testing system has changed
- [PR #1251](https://github.com/nf-core/rnaseq/pull/1251) - Replace deseq2qc paths
Expand Down
45 changes: 31 additions & 14 deletions subworkflows/local/align_star/nextflow.config
Original file line number Diff line number Diff line change
@@ -1,20 +1,37 @@
if (!params.skip_alignment && params.aligner == 'star_salmon') {
process {
withName: '.*:ALIGN_STAR:STAR_ALIGN|.*:ALIGN_STAR:STAR_ALIGN_IGENOMES' {
ext.args = { [
'--quantMode TranscriptomeSAM',
'--twopassMode Basic',
'--outSAMtype BAM Unsorted',
'--readFilesCommand zcat',
'--runRNGseed 0',
'--outFilterMultimapNmax 20',
'--alignSJDBoverhangMin 1',
'--outSAMattributes NH HI AS NM MD',
'--quantTranscriptomeBan Singleend',
'--outSAMstrandField intronMotif',
params.save_unaligned ? '--outReadsUnmapped Fastx' : '',
params.extra_star_align_args ? params.extra_star_align_args.split("\\s(?=--)") : ''
].flatten().unique(false).join(' ').trim() }
ext.args = {
Copy link
Member

Choose a reason for hiding this comment

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

Principle makes sense.

Would it make more sense to factor out maps and merge from there? Like the following (untested).

ext.args = {
    // Function to convert argument strings into a map
    def argsToMap(String args) {
        args.split("\\s(?=--)").collectEntries {
            def (key, value) = it.trim().split(/\s+/, 2)
            [(key): value]
        }
    }

    // Initialize the map with preset arguments
    def preset_args_map = argsToMap('''
        --quantMode TranscriptomeSAM
        --twopassMode Basic
        --outSAMtype BAM Unsorted
        --readFilesCommand zcat
        --runRNGseed 0
        --outFilterMultimapNmax 20
        --alignSJDBoverhangMin 1
        --outSAMattributes NH HI AS NM MD
        --quantTranscriptomeBan Singleend
        --outSAMstrandField intronMotif
        ${params.save_unaligned ? '--outReadsUnmapped Fastx' : ''}
    '''.trim())

    // Convert extra arguments into a map
    def extra_args_map = params.extra_star_align_args ? argsToMap(params.extra_star_align_args) : [:]

    // Merge maps: extra arguments override preset arguments
    def final_args_map = preset_args_map + extra_args_map

    // Convert the map back to a list and then to a single string
    final_args_map.collect { key, value -> "${key} ${value}" }.join(' ').trim()
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Either way is fine with me.

Since I do know neither Java nor Groovy (why is Nextflow not pythonic :-/ ?!?) I was just happy that I could nudge the AI into coming up with something that worked. But it is true, using a map is an elegant solution, because it avoids repeatedly looping over the preset arguments for each extra argument.

// Function to convert argument strings into a map
def argsToMap(String args) {
args.split("\\s(?=--)").collectEntries {
def (key, value) = it.trim().split(/\s+/, 2)
[(key): value]
}
}
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved

// Initialize the map with preconfigured values
def preset_args_map = argsToMap("""
--quantMode TranscriptomeSAM
--twopassMode Basic
--outSAMtype BAM Unsorted
--readFilesCommand zcat
--runRNGseed 0
--outFilterMultimapNmax 20
--alignSJDBoverhangMin 1
--outSAMattributes NH HI AS NM MD
--quantTranscriptomeBan Singleend
--outSAMstrandField intronMotif
${params.save_unaligned ? '--outReadsUnmapped Fastx' : ''}
""".trim())

// Consolidate the extra arguments
def final_args_map = preset_args_map + (params.extra_star_align_args ? argsToMap(params.extra_star_align_args) : [:])

// Convert the map back to a list and then to a single string
final_args_map.collect { key, value -> "${key} ${value}" }.join(' ').trim()
}

publishDir = [
[
path: { "${params.outdir}/${params.aligner}/log" },
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading