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

getSymbols.tiingo should return a Date index #350

Closed
ethanbsmith opened this issue Dec 11, 2021 · 6 comments
Closed

getSymbols.tiingo should return a Date index #350

ethanbsmith opened this issue Dec 11, 2021 · 6 comments

Comments

@ethanbsmith
Copy link
Contributor

Description

getSymbols.tiingo currently only return daily sampled date, but the index on the data is POSIXct
tm.stamps <- as.POSIXct(stock.data[, "date"], ...)

  1. this is inconsistent with other daily data source, which use a Date data type on the index
  2. using a POSIXct data type on daily data exposes the consumer to time zone issues

Expected behavior

return the index as a Date data type

joshuaulrich added a commit that referenced this issue Dec 11, 2021
We were using a POSIXct index for the results, while other getSymbols
methods use a Date index for daily data. And tiingo only provides
daily end of day data anyway.

We could convert to weekly, monthly, or yearly. But that functionality
doesn't exist now, so no one could be using it. And users can convert
to higher periodicity after the call to getSymbols().

Fixes #350.
@joshuaulrich
Copy link
Owner

Thanks for the report! Let me know what you think of the patch in the commit I just pushed.

@ethanbsmith
Copy link
Contributor Author

just checked the tiingo docs and they do support those other sampling frequencies. is there a reason to disallow them? would seem to be a potential breaking change. returning a Date index for those periodicities would match how getSymbols.yahoo works

image

rest looks good

@joshuaulrich
Copy link
Owner

joshuaulrich commented Dec 11, 2021

Good find regarding that argument! I didn't (and still couldn't) find that in the documentation. I added that functionality to the patch.

@ethanbsmith
Copy link
Contributor Author

@ethanbsmith
Copy link
Contributor Author

scratch that. i need to do some testing

@ethanbsmith
Copy link
Contributor Author

71978df looks good. thx for this!

@joshuaulrich joshuaulrich added this to the Release 0.4.20 milestone Mar 27, 2023
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