-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
ENH: Improved Yahoo finance api functionality #2795
Conversation
Hi, You've changed the signature of |
Hi @y-p, Thanks for pointing this out! I'd be glad to fix this. Just to be sure, is the concern with the signature mostly positional or keys (or both)? Being that the function now allows for multiple stock symbols, I thought it would be beneficial to have a name that reflected such, but perhaps this isn't good practice. The function signature was previously: get_data_yahoo(name=None, start=None, end=None, retry_count=3, pause=0) This PR contains changes as: get_data_yahoo(symbols=None, start=None, end=None, adjust_price=False,
ret_index=False, chunk=25, pause=0, **kwargs): Would a change to the following be sufficient? get_data_yahoo(name=None, start=None, end=None, retry_count=3, pause=0,
adjust_price=False, ret_index=False, chunk=25, pause=0,
**kwargs): When you can, let me know! Thanks! |
yep, keeping the old argument order and adding new ones at the end (all with defaults) Some network related tests are marked as @slow so make sure you run the full a note on this very issue was just added a couple of days ago: Thanks for contributing. |
Well, for back-compat, obviously both. admitadly it's rare to use named arguments there's a bunch of that in the tree, look around. |
Ah, grand. That all makes sense and that's just the type of feedback I needed. I had spent some time looking around, but hadn't caught that treatment of maintaining compatibility with older args via passing in **kwds with warning of deprecation. That's great. Also, kudos to that well written contributing doc, it helps a lot. I'll get these changes implemented, full tested, and pushed soon! Thanks again, @y-p! |
I think with that commit, all should be good for backwards compatibility. Let me know of any other thoughts or suggestions you might have, thank you! |
Sorry @y-p, are you referring to the previous default value in |
yep. |
While I found it convenient, looking around the tree it didn't seem like it was common practice. There is not a default equity to point towards, perhaps, similarly, there shouldn't be a default index? Also, I didn't want to seem particularly biased towards US markets. But, if you think it's a unique case where it's better with it, I'm certainly open to the idea.... |
Cherry-picked these commits. Thanks |
Added functionality of listing stock index components via query to yahoo finance
API. Expanded existing functionality of other yahoo finance features in
pandas.io.data.py
, in detail:get_component_yahoo()
, allowing for query of stock indexcomponents. Returns DataFrame containing list of component information for
index represented by
idx_sym
from yahoo. Result includes component symbol(ticker), exchange, and full name.
get_data_yahoo
, now allows for multiple stock symbols with inputof list-like object (tuple, list, Series), or DataFrame with stock symbols as index.
Includes convenience options for adjusting price and adding return index.
get_quote_yahoo
. Can now accept a single symbol (str) along withlist-like object (list, tuple, Series).
EDIT: A few typos.