fix: name of bamqc

[christopher-schroeder]

Description

The subcommand name was wrong in the description. Additionally add a region param fro target analyse.

QC

• I confirm that:

For all wrappers added by this PR,

• there is a test case which covers any introduced changes,
• input: and output: file paths in the resulting rule can be changed arbitrarily,
• either the wrapper can only use a single core, or the example rule contains a threads: x statement with x being a reasonable default,
• rule names in the test case are in snake_case and somehow tell what the rule is about or match the tools purpose or name (e.g., map_reads for a step that maps reads),
• all environment.yaml specifications follow the respective best practices,
• wherever possible, command line arguments are inferred and set automatically (e.g. based on file extensions in input: or output:),
• all fields of the example rules in the Snakefiles and their entries are explained via comments (input:/output:/params: etc.),
• stderr and/or stdout are logged correctly (log:), depending on the wrapped tool,
• temporary files are either written to a unique hidden folder in the working directory, or (better) stored where the Python function tempfile.gettempdir() points to (see here; this also means that using any Python tempfile default behavior works),
• the meta.yaml contains a link to the documentation of the respective tool or command,
• Snakefiles pass the linting (snakemake --lint),
• Snakefiles are formatted with snakefmt,
• Python wrapper scripts are formatted with black.
• Conda environments use a minimal amount of channels, in recommended ordering. E.g. for bioconda, use (conda-forge, bioconda, nodefaults, as conda-forge should have highest priority and defaults channels are usually not needed because most packages are in conda-forge nowadays).

[fgvieira]

Shouldn't target be an input file?
And can you also add a test using a target file?

[christopher-schroeder]

I thought about that. The problem is, that this parameter is optional. When developing a generic pipeline you would like to have this paramter for exomes and dont want to have it genomes. So you normally build a function. Something like

rule qualimap:
    input:
        bam="results/alignment/{sample}.bam",
        target=get_target

In the case of genomic data, get_target is None or "". But if defined as input, Snakemake will check the file for existence. But it cannot check for None or "". So I think the solution then would be

def qualimap_input(wc):
    ret = {"bam": "results/alignment/{wc.sample}.bam"}
    if target:
        ret["target"] = target
    return ret

rule qualimap:
    input:
        unpack(qualimap_input)

I am not convinced of either of the 2 options and in the end I decided to go for the params method. But if you prefer the other method you can convince me easily.

[fgvieira]

I think the way to go would be option 2 and have it as an input.

[johanneskoester]

I thought about that. The problem is, that this parameter is optional. When developing a generic pipeline you would like to have this paramter for exomes and dont want to have it genomes. So you normally build a function. Something like

rule qualimap:
    input:
        bam="results/alignment/{sample}.bam",
        target=get_target

In the case of genomic data, get_target is None or "". But if defined as input, Snakemake will check the file for existence. But it cannot check for None or "". So I think the solution then would be

def qualimap_input(wc):
    ret = {"bam": "results/alignment/{wc.sample}.bam"}
    if target:
        ret["target"] = target
    return ret

rule qualimap:
    input:
        unpack(qualimap_input)

I am not convinced of either of the 2 options and in the end I decided to go for the params method. But if you prefer the other method you can convince me easily.

It is also possible to return [] from the input function to indicate that the target is missing.

[fgvieira]

yes, it can also be option 1, if you return None when no target.
But it should be an input file

[github-actions]

This PR was marked as stale because it has been open for 6 months with no activity.