Refactors _run_dada from R to python

[Macabe222]

Description

Deletes run_dada.R and replaces it with _run_dada.py, using rpy2. Slightly faster on moving pictures demux.qza, 1:28 vs 1:33.
Addresses #172

AI Disclosure

• NO AI USED.
• AI USED.

[colinvwood]

waiting on #190

[colinvwood]

Note: Pseudo-pooling now uses DADA2’s native pool="pseudo" implementation, instead of two calls to dada on our end. That updates the error model between rounds, whereas run_dada.R reused the first learned model. This results in different, though desirable, output compared to the pre-refactor version.

[benjjneb]

The dada2 derepFastq function should not be called explicitly in this workflow. This is to avoid loading all samples into memory at the same time unnecessarily, which can be performance-breaking behavior on larger datasets.

benjjneb/dada2#919 (comment)
benjjneb/dada2#1095 (comment)

The R dada function should be passed a vector of file paths rather than the outpur of derepFastq.

Specific places where this performance hit will be felt are in the pseudo-pooling calls:

https://github.com/qiime2/q2-dada2/pull/197/changes/d638c023f9ab03f9a17061633b559ed7a37f63a1..0fc08b57a3e40a53b741dc459ee163cb6101c222#diff-dd7407f67ac9fa623337b0138c3539d679aa730b1ab132271bbf21a218822fbeR439
https://github.com/qiime2/q2-dada2/pull/197/changes/d638c023f9ab03f9a17061633b559ed7a37f63a1..0fc08b57a3e40a53b741dc459ee163cb6101c222#diff-dd7407f67ac9fa623337b0138c3539d679aa730b1ab132271bbf21a218822fbeR545
https://github.com/qiime2/q2-dada2/pull/197/changes/d638c023f9ab03f9a17061633b559ed7a37f63a1..0fc08b57a3e40a53b741dc459ee163cb6101c222#diff-dd7407f67ac9fa623337b0138c3539d679aa730b1ab132271bbf21a218822fbeR552

But the code for the regular operation of dada can also be simplified by removing the looping in the Q2 code:

https://github.com/qiime2/q2-dada2/pull/197/changes/d638c023f9ab03f9a17061633b559ed7a37f63a1..0fc08b57a3e40a53b741dc459ee163cb6101c222#diff-dd7407f67ac9fa623337b0138c3539d679aa730b1ab132271bbf21a218822fbeR568

Similarly, the calls to mergePairs can also take the filepaths instead of the output of derepFastq.

A test (probably non-automatable due to computational cost) to see if this is working correctly is to evaluate the memory usage of this workflow on increasing sample number. At larger numbers of samples, memory costs if derepFastq is being called on the whole dataset will go up linearly with sample number, while if samples are being processed one-at-a-time the memory cost will stay roughly constant with sample number.

[colinvwood]

Hello @benjjneb, thank you for the feedback. The memory concerns make sense; I will refactor to pass filepaths and let the R package be more intelligent wrt the derep objects.

[Oddant1]

Preliminary review. More to come.

One additional thing, what's with the changes to the test data?

[colinvwood]

Preliminary review. More to come.

One additional thing, what's with the changes to the test data?

Good question, they're for the pseudo pooling method only, and the reason is explained in my comment from a few weeks ago:

Note: Pseudo-pooling now uses DADA2’s native pool="pseudo" implementation, instead of two calls to dada on our end. That updates the error model between rounds, whereas run_dada.R reused the first learned model. This results in different, though desirable, output compared to the pre-refactor version.

[lizgehret]

Just chatted with @ebolyen about this, we'll get this reviewed shortly!

[VinzentRisch]

Hello
This looks like it was a lot of work! The refactored code looks very good and is a big improvement to the R file.
I ran some example data with denoise-paired and denoise-single and that works without problems.
See some comments below.

[VinzentRisch]

I think there should be unittests added for the helpers in this file.

[colinvwood]

I think these are covered by the tests added in #169