search

qiime2 / q2-composition

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

rename ancombc -> ancombc2 #112

Closed colinbrislawn closed 1 year ago

colinbrislawn commented 1 year ago

WIP, do not merge.

Simple change to method name to address warning.

This looks like an extension/improvement to ancombc function, but let's see if tests pass. https://github.com/FrederickHuangLin/ANCOMBC/blob/devel/R/ancombc2.R

colinbrislawn commented 1 year ago

unused arguments (formula = formula, tol = tol, max_iter = max_iter, conserve = conserve)

formula ->fix_formula tol and max_iter ->em_control = list(tol = tol, max_iter = max_iter) conserve is not used in ancombc2(). Looks like this is built into the new function?

' @param conserve logical. whether to use a conservative variance estimator for

' the test statistic. It is recommended if the sample size is small and/or

' the number of differentially abundant taxa is believed to be large.

' Default is FALSE.

Tests still use the old ancombc() function, which I'm keeping for now to see if new results match old results.

colinbrislawn commented 1 year ago

Error in colnames<-(*tmp*, value = "id") : attempt to set 'colnames' on an object with less than two dimensions

Looks like fit object returned by ancombc2() may be different.

ebolyen commented 1 year ago

Hey @colinbrislawn, yeah we realized pretty early that ancombc2 was not a drop-in replacement for ancombc, so we just finished work on adding ancombc and still need to do some exploration here for ancombc2.

cc @lizgehret

lizgehret commented 1 year ago

@colinbrislawn yeah echoing what @ebolyen mentioned above - these two methods are actually quite different. From the README in Huang's ANCOMBC repository, it looks like ANCOM-BC2 has a manuscript in preparation, so we had discussed waiting until there was a publication before adding this new method in.

lizgehret commented 1 year ago

Hey @colinbrislawn are you okay if we close this one out? Once a paper is published for ancombc2, we'll definitely look into adding that as another method in composition, but I don't know if this PR makes sense to keep open since it's not just a drag and drop replacement for ancombc.

colinbrislawn commented 1 year ago

Yes, I agree that a holistic PR is needed once we are ready.