databio / peppro

A modular, containerized pipeline for PRO-seq data processing
http://peppro.databio.org/
BSD 2-Clause "Simplified" License
10 stars 2 forks source link

name of `--umi` argument #21

Closed nsheff closed 5 years ago

nsheff commented 5 years ago

what is args.umi, really? Should it be renamed? It seems like it should mean "is there a UMI?", but even if false, we still treat UMIs.

instead it seems to be if you turn it on then it will use fastp to process UMIs. should it be --use-fastp-to-process-umis or something? --umi-fastp ?

I was confused because umi was set to False, but it was still processing UMIs.

jpsmith5 commented 5 years ago

Yes it is confusing. I prefer the umi-fastp variant myself but if that’s still unclear I’m not opposed to the alt.

nsheff commented 5 years ago

Alright next question: I don't understand why this is a flag argument, while the other similar 'choose a tool to do this' give you choices... And what should be the default?

is it the case that: if you do not use --umi-fast, then it will process UMIs with seqtk? then, right now seqtk is the default, then. Is that what we want?

In this case, why wouldn't this argument be:

--umi-processor with choices=['seqtk', 'fastp'] ?

nsheff commented 5 years ago

Wait, that appears to be wrong.. it's either "fastp", or use whatever you have as the trimmer, which can be 'seqtk' or 'fastx'.