cutadapt

[tverbeiren]

Description

Add cutadapt to the repo.

[tverbeiren]

Some food for thought:

Prefixes for FASTA files

-g and -G require an argument of the form file://<file>. But only if one wants to read from a FASTA file. In another pipeline, we solved this by letting the user specify a file and then in the script prepending file: to it. If we don't do it like this, Viash will not be able to verify the existence of the input file.

But what if the input file is not a FASTA file? Then the -g and -G take a string argument. Moreover, if we allow Viash to accept a string, how can we tell it needs to have the file: prefix added?

-a, -A, -b and -B behave the same.

Infrastructure argument

Cutadapt has a -j/--cores argument to specify the number of cores to run on. I suggest to not allow the user to set that for the Viash component. --cores 0 auto-detects.

What is the output?

Cutadapt allows to specify the output filenames of R1 and R2 separately. This is done using a form of variable substitution: -o "{name}_R1_001.fastq". I think, however, it makes more sense to provide an output directory, so that it does not matter anymore if cutadapt is run on paired or unpaired data when it comes to the output of the component.

Of course, downstream this will have to be managed.

[rcannood]

Some food for thought:

Prefixes for FASTA files

-g and -G require an argument of the form file://<file>. But only if one wants to read from a FASTA file. In another pipeline, we solved this by letting the user specify a file and then in the script prepending file: to it. If we don't do it like this, Viash will not be able to verify the existence of the input file.

But what if the input file is not a FASTA file? Then the -g and -G take a string argument. Moreover, if we allow Viash to accept a string, how can we tell it needs to have the file: prefix added?
-a, -A, -b and -B behave the same.

Alternatively, you could define two arguments:

arguments:
  - name: --adapter_file
    type: file
  - name: --adapter_string
    type: string

If both are defined, throw an error.

Infrastructure argument

Cutadapt has a -j/--cores argument to specify the number of cores to run on. I suggest to not allow the user to set that for the Viash component. --cores 0 auto-detects.

Makes sense!

What is the output?

Cutadapt allows to specify the output filenames of R1 and R2 separately. This is done using a form of variable substitution: -o "{name}_R1_001.fastq". I think, however, it makes more sense to provide an output directory, so that it does not matter anymore if cutadapt is run on paired or unpaired data when it comes to the output of the component.

Of course, downstream this will have to be managed.

will respond later ^^

[rcannood]

Great PR! It's always funny to see GitHub not wanting to show our config.vsh.yaml because it's too large :)

I requested some changes! 🙇

[rcannood]

Can we split up the output to separate output files?

CONTRIBUTING.md:

Step 8: Add arguments for output files

...
Note:
Preferably, these outputs should not be directores but files. For example, if a tool outputs a directory foo/ containing files foo/bar.txt and foo/baz.txt, there should be two output arguments --bar and --baz (as opposed to one output argument which outputs the whole foo/ directory).

Especially since cutadapt also doesn't rely in an output directory,

    ${par_json:+--json "${par_output}/report.json"} \
    --output "$par_output/{name}_R1_001.fastq" \
    --paired-output "$par_output/{name}_R2_001.fastq" \

Would need to be changed to

    ${par_json:+--json "${par_json}"} \
    --output "$par_output" \
    ${par_output_r2:+--paired-output "$par_output_r2"} \

or something to that extend

[tverbeiren]

Cutadapt implicitly creates a directory because it generates a number of output fastq files depending on the input data and the arguments provided. Hence the {name} variable. I would like to avoid that users have to set those 'variable' output paths explicitly.

[rcannood]

I agree that it would be cumbersome if the users would have to set the variable output paths explicitly. I now understand that cutadapt returns a variable number of outputs depending on how many adapters the user provides.

I'm still a bit on the fence because creating one big directory with all of the outputs makes it harder for downstream components to process the files.

Can we set something up with type: file, multiple: true such that, by default, the user can use --output to get all of the outputs in one directory, but if they provide paths with wildcards the fasta's / fastq's will be written to those paths instead?

• --output_dir: contains all of the files below
• --report_txt: report.txt
• --report_json: report.json
• --output: output_R1.fasta/q if paired, else output.fasta/q
• --output_r2: output_*_R2.fasta/q

[rcannood]

I just remembered that type: file, multiple: true, direction: output is currently broken and will be fixed in viash-io/viash#639. Will try to finish this PR ASAP.

[tverbeiren]

It's not really blocking I think because the bug appears in the VDSL3 module.

[tverbeiren]

Coming back to your suggestion, @rcannood:

I have a version in development that does what you suggest: take both an --output_dir as well as an --output argument. The first is just one directory, the second is multiple: true. This allows me to, for instance, run this in the tests:

"$meta_executable" \
  --output_dir out_test1 \
  --output out_test1/1_001.fasta \
  --output out_test1/unknown_001.fasta \
  ...

Or, alternatively --output out_test1/1_001.fasta;out_test1/unknown_001.fasta.

I agree it's nice to be able to capture the output explicitly rather than a directory, but it requires one to know the result of the cutadapt step prior to running it which will depend on the case at hand.

Ideally though, I would prefer to do something like:

"$meta_executable" \
  --output_dir out_test1 \
  --output out_test1/*.fast? \
  ...

This, however, does not work because of the way the executable is built (set -f disallows globbing):

# check whether required files exist
if [ ! -z "$VIASH_PAR_OUTPUT" ]; then
  IFS=';'
  set -f
  for file in $VIASH_PAR_OUTPUT; do
    unset IFS
    if [ ! -e "$file" ]; then
      ViashError "Output file '$file' does not exist."
      exit 1
    fi
  done
  set +f
fi

BTW: The way we deal with this in a Nextflow pipeline is by adding a map and glob inside the file() function of Nextflow.

Now, how to proceed?

1. Look into adapting the behaviour of Viash? (set -f)
2. Stick to what works with the individual output as illustrated above?
3. Just output the directory name and omit the individual output files?
4. (1) and (2)?

[rcannood]

Currently waiting for the next version of Viash to be released so we can get {type: file, multiple: true, direction: output} to work.

[rcannood]

Viash was updated to 0.9.0-rc3 in #51, so we can finally update this PR 😁

[tverbeiren]

Update: The --paired-output cutadapt argument is supported differently in this version. If paired input is provided, paired output is automatically written to $output_dir/<sample>_R[12]_001.... The output argument globs over the reads as well.

We could introduce --output_dir_r2 and --output_r2 but I don't see much sense in practical applications of that. It adds complexity and would only be useful when different reads need to end up in different directories.

WDYT @rcannood ?

[rcannood]

Minor change requested, otherwise LGTM

[tverbeiren]

It turns out .references should be under .links.