business-science / riingo

An R interface to the Tiingo stock price API
https://business-science.github.io/riingo/
Other
51 stars 9 forks source link

riingo prices fails if there is any invalid tickers #6

Closed zac-garland closed 5 years ago

zac-garland commented 5 years ago

Hi all,

You guys have done a great job with this package. There was one issue that I noticed which I wanted to bring to your attention. If there is a single invalid ticker, start_date, end_date, the entire call fails.

This could be problematic when calling 500 tickers, as the first 499 tickers might successfully download, but if there is one bad egg, it ruins the whole batch.

As an example:

tickers <- c("AAPL","IBM","F","BADTICKER")

riingo::riingo_prices(tickers)
#> Warning: 'glue::collapse' is deprecated.
#> Use 'glue_collapse' instead.
#> See help("Deprecated") and help("glue-deprecated").

#> Warning: 'glue::collapse' is deprecated.
#> Use 'glue_collapse' instead.
#> See help("Deprecated") and help("glue-deprecated").

#> Warning: 'glue::collapse' is deprecated.
#> Use 'glue_collapse' instead.
#> See help("Deprecated") and help("glue-deprecated").

#> Warning: 'glue::collapse' is deprecated.
#> Use 'glue_collapse' instead.
#> See help("Deprecated") and help("glue-deprecated").
#> Error: The ticker name, BADTICKER, is invalid or data is currently not available. Check ticker validity with is_supported_ticker().
#> Tiingo msg) Error: Ticker 'BADTICKER' not found

I tried to look through the source code, but found there were certain functions that weren't exported so I figured I'd post an issue.

Thanks, Zac

DavisVaughan commented 5 years ago

Thanks! I knew about this, but hadn't implemented any safe handling. I can add it in with purrr::safely(), but it might be a bit before I get to it. Thanks for reporting!

exploringfinance commented 5 years ago

Hi Davis. I think I found the issue. in the assertions (8), prices (1), quote (1), and url_constructor(1) file in the R folder I see a number of instances where it uses glue::collapse instead of the newer function in the tidyverse of glue_collapse. I made this fix and created a new tar file that I installed. The warning has stopped. Happy to share with you if you like

exploringfinance commented 5 years ago

Thanks Davis. This is a great package!

DavisVaughan commented 5 years ago

Thanks! Just sent 0.2.0 to cran, hopefully all goes smoothly