qiime2 / q2-cutadapt

BSD 3-Clause "New" or "Revised" License
3 stars 18 forks source link

IMP: Allow for demuxing of reads with mixed orientation #33

Closed Oddant1 closed 4 years ago

Oddant1 commented 5 years ago

Closes #11. First iteration of allowing for demuxing of reads with multiple orientations by exposing a mixed-orientation flag that, when provided, reruns the demux with the orientations swapped. This is a basic implementation of what is discussed in the comments on issue #11 and, though I have tested it manually, this initial commit adds no new unit tests.

Oddant1 commented 5 years ago

I know there is a TODO in that last commit, it should hopefully be resolved in the next one

Oddant1 commented 4 years ago

Pinging @thermokarst and @ebolyen because this came up on the forum again the other day, and I think it was basically done? I don't remember why it stalled out.

thermokarst commented 4 years ago

I don't remember why it stalled out.

Nor I, but this is on my todo list.

thermokarst commented 4 years ago

why did you decide to pass per_sample_sequences into _demux()?

Great question!

Previously, the helper _demux handled instantiating the directory format. For the mixed-orientation case though, I needed an easy way to write files into the directory format more than once. By injecting the instance in, I am now able to maintain the format's state outside of _demux.

Put another way, without that change, I would've wound up with two per_sample_sequences returned from my two calls to _demux. I would need to merge all the of the files in those two directories before returning from the Action, which is a huge pain. It is a pain because its not just as simple as copying the files into the same dir - you also have to concatenate any samples/files that are found in both per_sample_sequences directories.

Looking good, fellas.

Thanks so much for looking things over!