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

update module: diamond/blastp #7350

Open
4 tasks done
erikrikarddaniel opened this issue Jan 23, 2025 · 1 comment
Open
4 tasks done

update module: diamond/blastp #7350

erikrikarddaniel opened this issue Jan 23, 2025 · 1 comment
Assignees

Comments

@erikrikarddaniel
Copy link
Member

erikrikarddaniel commented Jan 23, 2025

Is there an existing module for this?

  • I have searched for the existing module

Is there an open PR for this?

  • I have searched for existing PRs

Is there an open issue for this?

  • I have searched for existing issues

Are you going to work on this?

  • If I'm planning to work on this module, I added myself to the Assignees to facilitate tracking who is working on the module

I have some issues with the current module, some easy to solve, some that need a discussion since they potentially change the interface of the module.

  1. I need to use the --include-lineage parameter supported by Diamond 2.10 but not the current 2.8. Easy to fix of course.
  2. Diamond supports reading gzipped files, so the unzipping of gzipped input seems unnecessary and a waste of space.
  3. Diamond also supports gzipping output files via --compress 1. Seems like a good feature to support, but is made difficult by the current third parameter being the file suffix which in turn decides the output format. My suggestion is to change the third param to the numerical output format and set the suffix from this instead. If $args contains --compress 1, a .gz can be added to the file suffix (except for .daa). (Overall, I also find the use of a suffix to decide the output format a bit too implicit for my taste. I definitely consider the blast output format a .tsv and would use that if I didn't know about the taxonomy (102) format.)
  4. In my experience, Diamond uses cpus quite efficiently and tends to require a lot of memory, so I'd suggest labelling the process process_high instead of medium.
  5. You're using find to get the path of the input .dmnd but I don't understand why since this is param two to the module.
  6. I think it would be good to include `${meta2.id}" in the default output file name as that's likely (it is in my case at least) the name of the database.

Update: I realize I need to match output channels from this module with the database channel (meta2) in my pipeline before calling the next module, i.e. join the output of this module with a channel that is indexed with database names (meta2.id). As I see this, I can either return both meta objects or add a field (e.g. db) containing meta2.id to the meta object before returning it. An alternative is perhaps to modify meta.id to contain the database name before calling this module.

I need a module like my description above for #metatdenovo, so I've made a local while we discuss. At the moment it looks like the below but hasn't been properly tested yet (i.e. WIP). I'm happy to contribute this to nf-core/modules if and when we agree.

    tag "$meta.id"
    label 'process_high'

    conda "${moduleDir}/environment.yml"
    container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
        'https://depot.galaxyproject.org/singularity/diamond:2.1.10--h5ca1c30_3':
        'biocontainers/diamond:2.1.10--h5ca1c30_3' }

   input:
    tuple val(meta) , path(fasta)
    tuple val(meta2), path(db)
    val outfmt
    val blast_columns

    output:
    tuple val(meta), path('*.blast*'), optional: true, emit: blast
    tuple val(meta), path('*.xml*')  , optional: true, emit: xml 
    tuple val(meta), path('*.txt*')  , optional: true, emit: txt 
    tuple val(meta), path('*.daa')   , optional: true, emit: daa 
    tuple val(meta), path('*.sam*')  , optional: true, emit: sam 
    tuple val(meta), path('*.tsv*')  , optional: true, emit: tsv 
    tuple val(meta), path('*.paf*')  , optional: true, emit: paf 
    path "versions.yml"              , emit: versions

    when:
    task.ext.when == null || task.ext.when

    script:
    def args = task.ext.args ?: ''
    def prefix = task.ext.prefix ?: "${meta.id}.${meta2.id}"
    def columns = blast_columns ? "${blast_columns}" : ''
    switch ( outfmt ) { 
        case 0:   out_ext = "blast"; break
        case 5:   out_ext = "xml";   break
        case 6:   out_ext = "txt";   break
        case 100: out_ext = "daa";   break
        case 101: out_ext = "sam";   break
        case 102: out_ext = "tsv";   break
        case 103: out_ext = "paf";   break
        default:
            outfmt = 6;
            out_ext = 'txt';
            log.warn("Unknown output file format provided (${out_ext}): selecting DIAMOND default of tabular BLAST output (txt)");
            break
    }   
    if ( args =~ /--compress\s+1/ ) out_ext += '.gz'
    """ 
    diamond \\
        blastp \\
        --threads ${task.cpus} \\
        --db $db \\
        --query ${fasta} \\
        --outfmt ${outfmt} ${columns} \\
        ${args} \\
        --out ${prefix}.${out_ext}

    cat <<-END_VERSIONS > versions.yml
    "${task.process}":
        diamond: \$(diamond --version 2>&1 | tail -n 1 | sed 's/^diamond version //')
    END_VERSIONS
    """ 
@mberacochea
Copy link
Contributor

Small comment, switch statements are "deprecated". Nextflow vscode docs - The Nextflow language specification does not support switch statements. Use if-else statements instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants