ropensci / bold

Interface to the Bold Systems barcode webservice
https://docs.ropensci.org/bold
Other
17 stars 11 forks source link

fix case when response contains empty list #32

Closed fmichonneau closed 7 years ago

fmichonneau commented 7 years ago

When doing:

bold_tax_id(321215, dataTypes="stats")

I get

Error in (function (..., row.names = NULL, check.rows = FALSE, check.names = TRUE,  : 
  arguments imply differing number of rows: 1, 0

Looking at the response from the API, I saw:

Browse[2]> out
$publicspecies
[1] 0

$publicbins
[1] 0

$publicmarkersequences
list()

$publicrecords
[1] 0

$publicsubspecies
[1] 0

$specimenrecords
[1] "15"

$sequencedspecimens
[1] "54"

$barcodespecimens
[1] "5"

$species
[1] "1"

$barcodespecies
[1] "1"

The proposed change in the code tries to deal with these situations where one the slots contains an empty list. I'm not entirely sure it's the best approach as I'm not too familiar with the other kinds of output this API provide, but all the tests pass.

codecov-io commented 7 years ago

Current coverage is 79.79% (diff: 100%)

Merging #32 into master will increase coverage by 0.31%

@@             master        #32   diff @@
==========================================
  Files             8          8          
  Lines           190        193     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            151        154     +3   
  Misses           39         39          
  Partials          0          0          

Powered by Codecov. Last update 85e0747...f4676ce

fmichonneau commented 7 years ago

actually, it doesn't quite work... as it still fails if more than one data type is asked (ie bold_tax_id(321215, dataTypes="basic,stats")). I'll revisit this in a couple of hours

sckott commented 7 years ago

Thanks! Hmm, maybe just Filter(length, ll) to remove zero length elements, like

(ll <- list(publicspecies = 0,  publicmarkersequences = list()))
#> $publicspecies
#> [1] 0
#> 
#> $publicmarkersequences
#> list()
#> 
Filter(length, ll)
#> $publicspecies
#> [1] 0

Can you add a test too in test-bold_tax_id ?

fmichonneau commented 7 years ago

Thanks Scott. I generalized the code so that we can deal with the last example I gave. I added the tests. Let me know what you think.

sckott commented 7 years ago

LGTM, thanks lots @fmichonneau