emmanuelparadis / ape

Analysis of Phylogenetics and Evolution
https://emmanuelparadis.github.io/
GNU General Public License v2.0
53 stars 11 forks source link

rtt function no longer handles NAs in dates #73

Closed brj1 closed 1 year ago

brj1 commented 1 year ago

The rrt function performs root-to-tip regression on trees. In previous versions, you could put NA in the dates parameter to censor tips from the regression. However, this does not seem to be possible anymore. I can make a PR to fix this, if you want.

emmanuelparadis commented 1 year ago

Would a line like this:

if (anyNA(tip.dates)) stop("missing values not allowed in 'tip.dates'")

at the start of rtt() be enough? I can add a note in rtt.Rd too.

brj1 commented 1 year ago

Could you add the ability to censor tip dates with NAs back? I need this functionality for some of the software that I maintain.

emmanuelparadis commented 1 year ago

You mean change nothing with respect to the current version?

brj1 commented 1 year ago

The problem is an update in R. Consider the following code:

library(ape)
set.seed(0)
tree <- rtree(5)
dates <- c(1, 2, 3, NA, NA)
rtt(tree, dates)

In R version 4.2.2 it gives an error:

Error in cor(y, x) : incompatible dimensions
In addition: There were 50 or more warnings (use warnings() to see the first 50)

In R version 4.0.5 it roots the tree.

tree

The issue is that the cor() function changed in the latest version of R so that by default NA are included in he correlation calculation where previously they would be censored.

The fix is fairly simple. Just change line 31 of rtt.R to:

cor(y, x, use = "complete.obs")
emmanuelparadis commented 1 year ago

Fixed and pushed.