Review cdhit_wrapper.R and reduce_sequence.R

[renewiegandt]

1. Code:

   1. I saw you added your information to the epilogue of optparse. But I would add "@author" and "@contact" to the comment block referring to the function.
   2. Could not find anything else that I would change.

2. Functionality:

   1. I did no test runs so far. Will be added later.

[renewiegandt]

I tried running your script without optparse installed.
Error in library("optparse") : there is no package called ‘optparse’
Maybe you could catch the error and install the package if its not installed or throw your own, more clear error message.

[renewiegandt]

Tried breaking the script further but everything seems fine.
Output looks like its supposed to look.
Not going to validate the output because we sufficiently checked the output beforehand.

[renewiegandt]

Quick and easy!

[renewiegandt]

While checking reduce_sequence.R I found a few more things.
If the script expects a double and it gets a character it does not throw an error only a warning.

Rscript cdhit_wrapper.R -i /mnt/agnerds/masterjlu2018/2.1clustering/testfiles/6motifs_hg19/six_motiv_hg19_with_seq.bed -o /mnt/agnerds/Rene.Wiegandt/10_Master/tmp/test_reduce.bed -c adsfasdf
Loading required package: optparse
Warning message:
In value[[3L]](cond) : double expected, got “adsfasdf”
/mnt/workspace1/rene.wiegandt/work/conda/masterenv-f17511358e7a5c5bfa9e7b862c4f9523/bin/cd-hit-est
Loading bed.
Convert to fasta.
Clustering.
================================================================
Program: CD-HIT, V4.7 (+OpenMP), Jul 13 2018, 17:17:44
Command: cd-hit-est -i converted_bed.fasta -o cdhit_output -c
         adsfasdf -A 8 -T 1 -G 0 -b 20 -M 800 -n 3 -l 5 -s 0 -S
         999999 -aL 0 -AL 99999999 -aS 0 -AS 99999999 -uL 1 -uS
         1 -U 99999999 -g 1 -r 0 -match 2 -mismatch -2 -gap -6
         -gap-ext -1 -sc 1 -d 0

Started: Thu Jan  3 09:46:27 2019
================================================================
                            Output
----------------------------------------------------------------

Fatal Error:
invalid clstr threshold, should >=0.8
Program halted !!

Is it possible to throw an error instead of a warning?

---

Edit: Going to do the review on reduce_sequence.R on this issue. Should be easier for you.

[renewiegandt]

One example:
In function 'significant_kmer'

if (motif_occurrence <= 0) {
    stop("Motif_occurrence must be a numeric value above 0!")
}

You should/could check if motif _occurrence is numeric.

("asdasdasdfasf" <= 0) equals false

Would be solved by changing the Warnings from optparse to errors (if possible). If not you should check every parameter on its type at the beginning of the script.
Another example:
I set -c as random string which does not throw an error.

[HendrikSchultheis]

So far I haven't found an optparse solution for this. And I also cannot suppress the type warning.

In fact at least for cdhit_wrapper.R and all it's cdhit related arguments I would say this isn't necessary as cdhit aborts with an error message. (Also it would be really cumbersome to add all those checks.)

For all the other parameter I agree there should be at least a type check. What do you think should I add argument checks only for arguments that are not forwarded to tools or to everything?

[renewiegandt]

It should be enough to do it only for the arguments, which are not forwarded to the tools.