ropensci / taxizedb

Tools for Working with Taxonomic SQL Databases
Other
30 stars 7 forks source link

children test failing #45

Open sckott opened 5 years ago

sckott commented 5 years ago

the test at https://github.com/ropensci/taxizedb/blob/master/tests/testthat/test-children.R#L10-L13 is failing

I see

taxize::children(3701, db='ncbi')
#> $`3701`
#>    childtaxa_id                                                     childtaxa_name childtaxa_rank
#> 1       1837063                         Arabidopsis thaliana x Arabidopsis halleri        species
#> 2       1746102                                             Arabidopsis sp. hda9-2        species
#> 3       1547873                                           Arabidopsis sp. NH-2014a        species
#> 4       1547872                                              Arabidopsis umezawana        species
#> 5       1328956 (Arabidopsis thaliana x Arabidopsis arenosa) x Arabidopsis suecica        species
#> 6       1240361                         Arabidopsis thaliana x Arabidopsis arenosa        species
#> 7        869751        Arabidopsis thaliana x Arabidopsis halleri subsp. gemmifera        species
#> 8        869750                          Arabidopsis thaliana x Arabidopsis lyrata        species
#> 9        412662                                            Arabidopsis pedemontana        species
#> 10       378006                         Arabidopsis arenosa x Arabidopsis thaliana        species
#> 11       347883                                              Arabidopsis arenicola        species
#> 12       302551                                              Arabidopsis petrogena        species
#> 13        97980                                               Arabidopsis croatica        species
#> 14        97979                                            Arabidopsis cebennensis        species
#> 15        81970                                                Arabidopsis halleri        species
#> 16        59690                                             Arabidopsis kamchatica        species
#> 17        59689                                                 Arabidopsis lyrata        species
#> 18        45251                                               Arabidopsis neglecta        species
#> 19        45249                                                Arabidopsis suecica        species
#> 20        38785                                                Arabidopsis arenosa        species
#> 21        29726                                                    Arabidopsis sp.        species
#> 22         3702                                               Arabidopsis thaliana        species
#> 
#> attr(,"class")
#> [1] "children"
#> attr(,"db")
#> [1] "ncbi"

taxizedb::children(3701, db='ncbi')
#> $`3701`
#>    childtaxa_id                                                     childtaxa_name childtaxa_rank
#> 1       1837063                         Arabidopsis thaliana x Arabidopsis halleri        species
#> 2       1547872                                              Arabidopsis umezawana        species
#> 3       1328956 (Arabidopsis thaliana x Arabidopsis arenosa) x Arabidopsis suecica        species
#> 4       1240361                         Arabidopsis thaliana x Arabidopsis arenosa        species
#> 5        869750                          Arabidopsis thaliana x Arabidopsis lyrata        species
#> 6        412662                                            Arabidopsis pedemontana        species
#> 7        378006                         Arabidopsis arenosa x Arabidopsis thaliana        species
#> 8        347883                                              Arabidopsis arenicola        species
#> 9        302551                                              Arabidopsis petrogena        species
#> 10        97980                                               Arabidopsis croatica        species
#> 11        97979                                            Arabidopsis cebennensis        species
#> 12        81970                                                Arabidopsis halleri        species
#> 13        59690                                             Arabidopsis kamchatica        species
#> 14        59689                                                 Arabidopsis lyrata        species
#> 15        45251                                               Arabidopsis neglecta        species
#> 16        45249                                                Arabidopsis suecica        species
#> 17        38785                                                Arabidopsis arenosa        species
#> 18         3702                                               Arabidopsis thaliana        species
#> 
#> attr(,"class")
#> [1] "children"
#> attr(,"db")
#> [1] "ncbi"
:p> packageVersion('taxize')
[1] ‘0.9.4.9914’

:p> packageVersion('taxizedb')
[1] ‘0.1.7.9602’
arendsee commented 5 years ago

@sckott Looks like an issue with the ambiguous flag:

These are equal:

taxizedb::children(3701, db='ncbi', ambiguous=TRUE)
taxize::children(3701, db='ncbi', ambiguous=TRUE)

And so are these:

taxizedb::children(3701, db='ncbi', ambiguous=FALSE)
taxize::children(3701, db='ncbi', ambiguous=FALSE)

But not these:

taxizedb::children(3701, db='ncbi')
taxize::children(3701, db='ncbi')

Judging by the output above, in taxize the default is ambiguous=TRUE and in taxizedb it is ambiguous=FALSE.

I think something is mixed up on the taxize side. According to ?taxize::ncbi_children, ambiguous is FALSE by default. Yet somewhere the default appears to be overridden.

arendsee commented 5 years ago

What do you think about this set of test:

  # they agree in the ambiguous case
  expect_equal(
    taxizedb::children(3701, db='ncbi', ambiguous=TRUE),
    taxize::children(3701, db='ncbi', ambiguous=TRUE)
  )

  # they agree in the unambiguous case
  expect_equal(
    taxizedb::children(3701, db='ncbi', ambiguous=FALSE),
    taxize::children(3701, db='ncbi', ambiguous=FALSE)
  )

  # the defaults agree
  expect_equal(
    taxizedb::children(3701, db='ncbi'),
    taxize::children(3701, db='ncbi')
  )

Currently, the last test fails, but I think the default behavior of taxize and taxizedb should be the same.

maelle commented 1 year ago

unarchiving it thanks to @stitam :smile_cat: