-
Notifications
You must be signed in to change notification settings - Fork 52
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
[Enhancement] Add new daily and IEX methods following the Tiingo API more closely #527
Comments
Sounds good to me, personally. I finally have a few moments to start looking into this code a bit more and see where I can help with the project. |
Hi @2xmm , thanks for filing this and organizing this proposal based on our discussions in this recent PR. I think adding these two new methods you propose ( If it turns out people have a strong need for the JSON format's timezone offsets for intraday data, they can still use the non-convenience mechanism (i.e. |
@GenusGeoff Feel free to work on this if you are interested. |
@2xmm This looks like it's already implemented in the most recent code with the switch "fmt='csv'". Great work to @hydrosquall or to whomever took this one up. |
Hi @GenusGeoff ! @2xmm generously implemented the I think the change request described in this issue is specifically for adding 2 new methods to the library, |
Please forgive my lack of understanding on the pull/fork/merge type thing. I've included some changes to api.py below. They're extremely rough but they do what is requested. Comments and all might need to be cleaned up a bit, though. It's a simple fix that just borrows much of the code from the existing get_dataframe() function. `####################### BEGIN Modifications (GenusGeoff)
## Get IEX Data
### End of Modifications (GenusGeoff)` |
Added functionality in rough form as requested in "[Enhancement] Add new daily and IEX methods following the Tiingo API more closely hydrosquall#527" from tiingo-python
This is a bit more to add the IEX afterHours quotes to this code. BTW--Apparently, Tiingo requires True and False to be lowercase in the request which is incompatible with Python. A possible workaround might be to create a literal variable, e.g. true, false = (True, False); but I've no idea what unintended consequences might arise. `## Get IEX Data
` |
Hi @GenusGeoff, Glad you were able to make a contribution to this project. It's a great exercise to work through submitting a pull request and you can find out how to do so in the |
Description
Tiingo daily data and IEX intraday data are both being returned by the
get_dataframe
method but I propose they would be better off with their own methods. This would follow the Tiingo API more closely and users would be more aware of the source of the underlying data.I suggest adding the method names
get_daily_data
andget_iex_data
. The suffix_data
is more appropriate in this case because if ametric_name
is passed then the method returns apandas.Series
not apandas.DataFrame
.I recommend only returning data in
csv
format from these new methods since this return type is so much faster thanjson
. Ifjson
is included as a return type, test of equality betweencsv
andjson
should be included. I've noticed that intradaycsv
response data coming from Tiingo contains a timezone offset whilejson
response data does not contain a timezone offset. I would recommend using the same offset as the one returned by thecsv
response for simplicity. Again, the decision about which offset to use could be avoided by only allowing these methods to returncsv
data.@hydrosquall Looking forward to your thoughts.
The text was updated successfully, but these errors were encountered: