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

ENH: Improved Yahoo finance api functionality #2795

Closed
wants to merge 3 commits into from

Conversation

@nehalecky
Copy link
Contributor

commented Feb 4, 2013

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:

  • Created new function get_component_yahoo(), allowing for query of stock index
    components. Returns DataFrame containing list of component information for
    index represented by idx_sym from yahoo. Result includes component symbol
    (ticker), exchange, and full name.
  • Refactored get_data_yahoo, now allows for multiple stock symbols with input
    of list-like object (tuple, list, Series), or DataFrame with stock symbols as index.
    Includes convenience options for adjusting price and adding return index.
  • Refactored get_quote_yahoo. Can now accept a single symbol (str) along with
    list-like object (list, tuple, Series).
  • Expanded test coverage to the new features.

EDIT: A few typos.

@ghost

This comment has been minimized.

Copy link

commented Feb 4, 2013

Hi,

You've changed the signature of get_data_yahoo so old code might break.
Is there no way to make your improvments backwards-compatible?

@nehalecky

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2013

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!

@ghost

This comment has been minimized.

Copy link

commented Feb 4, 2013

yep, keeping the old argument order and adding new ones at the end (all with defaults)
should do it, as long as the function body still behaves the same.

Some network related tests are marked as @slow so make sure you run the full
suite with test.sh (or manually use nose to run them)

a note on this very issue was just added a couple of days ago:
https://github.com/pydata/pandas/blob/master/CONTRIBUTING.md

Thanks for contributing.

@ghost

This comment has been minimized.

Copy link

commented Feb 4, 2013

Well, for back-compat, obviously both. admitadly it's rare to use named arguments
for the first arg in a call. but if you must change an arg name, detect the original's use
in **kwds and issue a deprecation warning and handle it so the call still succeeds.

there's a bunch of that in the tree, look around.

@nehalecky

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2013

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!

@nehalecky

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2013

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!

@nehalecky

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2013

Sorry @y-p, are you referring to the previous default value in get_component_yahoo, '^DJI'?

@ghost

This comment has been minimized.

Copy link

commented Feb 5, 2013

yep.

@nehalecky

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2013

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....

@ghost ghost assigned wesm Feb 7, 2013

@wesm

This comment has been minimized.

Copy link
Member

commented Feb 10, 2013

Cherry-picked these commits. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.