ropensci / taxa

taxonomic classes for R
https://docs.ropensci.org/taxa
Other
48 stars 12 forks source link

Databases stored as character cause error for print methods #42

Closed zachary-foster closed 7 years ago

zachary-foster commented 7 years ago

This affects taxon_id, taxon_rank, and taxon_name. @sckott, at one point I think we discussed allowing the taxon database field of the above functions to be a character vector matching a name in database_list. Doing that currently causes an error.

I think allowing characters would save a lot of RAM for large datasets, since each taxon object could have 3 database objects, each with the tables mentioned in issue #40. Does that work for you?

<TaxonName> Poa
 Show Traceback

 Rerun with Debug
 Error in self$database$name : $ operator is invalid for atomic vectors > taxon_name("Poa", database = ncbi)
<TaxonName> Poa
  database: ncbi
> sessionInfo()
R version 3.3.1 (2016-06-21)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 16.04.1 LTS

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] taxa_0.0.4.9105

loaded via a namespace (and not attached):
 [1] magrittr_1.5     assertthat_0.2.0 R6_2.2.0         DBI_0.6-1        tools_3.3.1      dplyr_0.5.0      tibble_1.3.0    
 [8] Rcpp_0.12.10     knitr_1.15.1     jsonlite_1.4    
zachary-foster commented 7 years ago

To be clear, I think we should still allow for the value to be a TaxonDatabase object, but if it is a character, it has to match a name in database_list. We could also add function that lets the user add a custom database to database_list, so that they can use characters for custom databases

sckott commented 7 years ago

yeah, that sounds good

zachary-foster commented 7 years ago

Nice, I think this is done then