Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getQuote fails if any symbols are not found #279

Closed
ethanbsmith opened this issue Oct 14, 2019 · 3 comments
Closed

getQuote fails if any symbols are not found #279

ethanbsmith opened this issue Oct 14, 2019 · 3 comments
Assignees

Comments

@ethanbsmith
Copy link
Contributor

ethanbsmith commented Oct 14, 2019

Description

getQuote.yahoo fails if yahoo does not recognize any of the symbols. This is particularly problematic when a very large list of quotes is requested as identifying the offending symbol is near impossible. Since the identifying Symbol may be perfectly valid, but yahoo just doesn't recognize it. eg:

DEENF which is a valid OTC ticker:
https://www.otcmarkets.com/stock/DEENF/overview

Expected behavior

ideally getQuote would return the values for the found symbols and NAs for the ones that weren't found

Minimal, reproducible example

> getQuote(c("DE", "DEENF"), src = "yahoo")
Error in `.rowNamesDF<-`(x, value = value) : invalid 'row.names' length

Session Info

> sessionInfo()
R version 3.5.3 (2019-03-11)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18362)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252    LC_MONETARY=English_United States.1252 LC_NUMERIC=C                           LC_TIME=English_United States.1252    

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

other attached packages:
 [1] doParallel_1.0.14    iterators_1.0.11     foreach_1.5.1        data.table_1.12.2    curl_3.3             rvest_0.3.4          xml2_1.2.0           quantmod_0.4-15      TTR_0.23-4          
[10] xts_0.11-2           zoo_1.8-6            RODBC_1.3-15         plotrix_3.7-6        RevoUtils_11.0.3     checkpoint_0.4.4     RevoUtilsMath_11.0.0

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.1       magrittr_1.5     lattice_0.20-38  R6_2.3.0         stringr_1.4.0    httr_1.4.0       tools_3.5.3      grid_3.5.3       selectr_0.4-1    codetools_0.2-16 stringi_1.4.3    compiler_3.5.3  
[13] jsonlite_1.5 
@ethanbsmith
Copy link
Contributor Author

I think the main jist of the fix is to replace line 59:
sq <- response$quoteResponse$result
with:
sq <- merge.data.frame(data.frame(symbol = Symbols), response$quoteResponse$result, by = "symbol", all = T)

if I can find some time, I'll try cruft up a pr

@joshuaulrich
Copy link
Owner

Thanks for the report and patch! Your suggestion is very close, but wouldn't quite work because Symbols would be "DE,DEENF" at line 59. You would need to do it after line 80.

It might also be good to only do the merge conditioned on NROW(sq) != length(Symbols). Also, it's not good practice to call methods directly, so I would change merge.data.frame() to just be merge(), and I would add a stringsAsFactors = FALSE to the data.frame constructor, just to be safe.

So something like:

if (length(Symbols) != NROW(sq)) {
  sq <- merge(data.frame(symbol = Symbols, stringsAsFactors = FALSE),
              sq, by = "symbol", all = TRUE)
}

Please double-check that works, and make a PR when you're ready. Thanks!

@joshuaulrich
Copy link
Owner

This was fixed in #282. Closing.

@joshuaulrich joshuaulrich self-assigned this Nov 18, 2019
joshuaulrich pushed a commit that referenced this issue Nov 24, 2019
For some reason, calling fromJSON() directly throws errors with long
symbol lists. Thanks to @ethanbsmith for the report and patch.

See #279.
joshuaulrich added a commit that referenced this issue Nov 24, 2019
Add a 'Symbol' column the output of all the 'method' functions.

Remove initial uppercase and 'std.cols' handling in 'tiingo' method.
These weren't used, and were deleted to be easier to maintain and to
avoid future confusion.

See #279. Closes #288.

Squashed commit of the following:

commit 0541c4d
Author: Joshua Ulrich <josh.m.ulrich@gmail.com>
Date:   Sun Nov 24 14:44:21 2019 -0600

    Revert "Update getQuote.R"

    This reverts commit 5ff4cd6.

commit 5ff4cd6
Author: ethanbsmith <24379655+ethanbsmith@users.noreply.github.com>
Date:   Sat Nov 23 13:49:12 2019 -0700

    Update getQuote.R

    fixed column name

commit 28b581b
Author: Joshua Ulrich <josh.m.ulrich@gmail.com>
Date:   Sat Nov 23 11:18:59 2019 -0600

    Avoid implicit conversion and overwriting args

    Also fix typo in Tiingo column name and address whitespace/formatting.

commit 23d5f16
Author: ethanbsmith <24379655+ethanbsmith@users.noreply.github.com>
Date:   Fri Nov 22 16:00:23 2019 -0700

    Update getQuote.R

commit 2223a5d
Author: ethanbsmith <24379655+ethanbsmith@users.noreply.github.com>
Date:   Fri Nov 22 15:58:45 2019 -0700

    Update tests.R

    added test for batch size and correct row labeling

commit 8f83d8c
Author: ethanbsmith <24379655+ethanbsmith@users.noreply.github.com>
Date:   Fri Nov 22 15:50:31 2019 -0700

    Update getQuote.R

    moved coumn name and missing row handling into master function

commit 41cc351
Author: ethanbsmith <24379655+ethanbsmith@users.noreply.github.com>
Date:   Fri Nov 22 15:10:03 2019 -0700

    explicit curl call

    explicitly call curl::curl. for some unknown reasosn the direct call
    to fromJSON causes erros with long symbol lists
joshuaulrich added a commit that referenced this issue Nov 25, 2019
Quotes were returned with Symbols sorted in lexical order. That made
it hard to write tests. It also seems reasonable to expect that the
output would be in the same order as the input.

See #279.
@joshuaulrich joshuaulrich added this to the Release 0.4-16 milestone Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants