search

liamrevell / phytools

GNU General Public License v3.0
207 stars 56 forks source link

scoping problem in fastAnc, phytools_reroot; breaks when latest treeio is loaded #47

Closed pbradz closed 5 years ago

pbradz commented 5 years ago

Thanks very much for your work on this package!

Description of problem: phytools::fastAnc seems to call the function root, which appears to typically be resolved to ape::root.phylo. However, the package treeio (now necessary for using ggtree) now defines its own function root.phylo which is not compatible, and which fails if the new root is equal to ((# tips) + 1). fastAnc therefore throws a non-intuitive error if treeio is loaded. Ironically, treeio:::root.phylo actually wraps phytools::reroot, but because phytools::reroot also calls root with no scoping, it ends up circling back to treeio:::root.phylo.

Things I've tried: Manually setting root.phylo <- ape::root.phylo doesn't seem to override what happens when I call fastAnc. Loading treeio first also doesn't work.

Workaround: I don't have a good one, other than redefining fastAnc in my own package and changing the scoping.

Suggested fix: replace root with ape::root.phylo in fastAnc and reroot (and ideally also wherever else is appropriate in phytools).

liamrevell commented 5 years ago

Thanks for bringing this to my attention. I believe it has now been resolved. (Please post a new issue if this is not the case.) Much appreciated.