Suggestion: L table is a data frame that has column names

[richelbilderbeek]

Hi @xl0418, thanks for the dododo package!

In the work by me and @Giappo, I find a function like this:

bd_phylo_2_l_table <- function(phylo) {
  l_table <- dododo::phylo2L(phylo) # nolint
  colnames(l_table) <- c("birth_time", "parent", "id", "death_time")
  return(l_table)
}

All that function does, is convert the L table into a data frame and add column names.

I think this is superior over the current state of affairs, in which an L table is a matrix. Matrices only apply to data of the same type, where a species/parent ID and time of birth/death clearly have different units.

In effect, I suggest the following test to be added:

test_that("L table is a data frame that has column names", {

  phylogeny <- ape::read.tree(text = "((t1:1, t3:1):1, t2:2);")
  l_table <- phylo2L(phylogeny)
  expect_true(is.data.frame(l_table))
  col_names <- colnames(l_table)
  expected <- c("birth_time", "parent", "id", "death_time")
  expect_equal(col_names, expected)
})

I am rather sure @Giappo would volunteer to add this and the code to pass the test, as he was the original author of the first function.

Will you accept this suggestion yes/no?

[xl0418]

Yes, agree. best, Liang -- Liang Xu PhD Student Theoretical Research in Evolutionary Life Sciences (TRÊS) Groningen Institute for Evolutionary Life Sciences Box 11103 9700 CC Groningen

…

在 2019年1月7日，07:23，Richel Bilderbeek ***@***.***> 写道： Hi @xl0418, thanks for the dododo package! In the work by me and @Giappo, I find a function like this: bd_phylo_2_l_table <- function(phylo) { l_table <- dododo::phylo2L(phylo) # nolint colnames(l_table) <- c("birth_time", "parent", "id", "death_time") return(l_table) } All that function does, is convert the L table into a data frame and add column names. I think this is superior over the current state of affairs, in which an L table is a matrix. Matrices only apply to data of the same type, where a species/parent ID and time of birth/death clearly have different units. In effect, I suggest the following test to be added: test_that("L table is a data frame that has column names", { phylogeny <- ape::read.tree(text = "((t1:1, t3:1):1, t2:2);") l_table <- phylo2L(phylogeny) expect_true(is.data.frame(l_table)) col_names <- colnames(l_table) expected <- c("birth_time", "parent", "id", "death_time") expect_equal(col_names, expected) }) I am rather sure @Giappo would volunteer to add this and the code to pass the test, as he was the original author of the first function. Will you accept this suggestion yes/no? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.