Skip to content

PR to pandas #3

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

Closed
twiecki opened this issue Jan 12, 2015 · 4 comments
Closed

PR to pandas #3

twiecki opened this issue Jan 12, 2015 · 4 comments

Comments

@twiecki
Copy link

twiecki commented Jan 12, 2015

What are you thoughts on contributing this to Pandas? I'm sure they'd be appreciative.

@femtotrader
Copy link
Owner

They (or anyone) can take the code if they want. You can also fork and send a PR if you want.

but see this before pandas-dev/pandas#8713 and
pandas-dev/pandas#8961
see also pandas-dev/pandas#6456

syntax is slightly different between this version of "DataReader" (which is an object) and Pandas one (which is a function)

see for example test:

https://github.com/femtotrader/pandas_datareaders/blob/master/tests/test_datareader_google_finance_daily.py

New version:

df= DataReader("GoogleFinanceDaily", expire_after=expire_after).get(symbol, start, end)

instead of

df = web.DataReader(symbol, 'google', start, end)

so Pandas doc needs to be modified accordingly

Moreover I wonder if such a PR was accepted if we need to also keep previous syntax and warm users that this syntax is deprecated

I've done one file per DataReader and one file per DataReader test.
I think it's cleaner.

Such a PR will add requests as a requirement for Pandas (see pandas-dev/pandas#8713 @TomAugspurger @jreback and some others pandas-dev/pandas#6456 @cpcloud )
so I feel quite alone ;-)

@twiecki
Copy link
Author

twiecki commented Jan 12, 2015

Thanks @femtotrader. Seems like they have pretty good arguments for keeping this separate which is fine. This could easily be an optional dependency for zipline.

@twiecki twiecki closed this as completed Jan 12, 2015
@femtotrader
Copy link
Owner

I also see "requests_cache is relatively transparent, but I am not sure on how unreasonable it is.

If I have a function that gets historical data from yahoo, I feel like the caching should be done by choice by the user."

I don't see any "good arguments" for not using requests in fact. But that's probably a misunderstanding of english from my side.

In fact the "problem" of DataReaders is not clear see pandas-dev/pandas#9195

I really think that we should DISCUSS about it here: pandas-dev/pandas#8961

Thanks for closing this issue ;-)

@twiecki
Copy link
Author

twiecki commented Jan 13, 2015

Well, seems like they say it's out of scope. But I agree that it should either be completely excluded or extended then.

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