-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
REV/MAINT: Switching back to Yahoo (temp) to deal with Google #1972
Conversation
@@ -356,93 +352,6 @@ def _load_cached_data(filename, first_date, last_date, now, resource_name, | |||
return None | |||
|
|||
|
|||
def _load_raw_yahoo_data(indexes=None, stocks=None, start=None, end=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should've been removed in #1812
Thank you. This is causing issues to a lot of people. |
@richafrank any thoughts on how to deal with this? Passing Py27 and Py35 but not Py34. No |
Google limited their API to only 251 days worth of data, most likely as a result of the change that Yahoo had made a few months ago. `pandas-datareader` made a fix for the Yahoo data reader so I'm making this PR to move back to Yahoo until we can do something better. Ideally we'll have a more robust solution before we do another release, so users will have to install latest Zipline master if this is merged.
Previously we've added a recipe for the missing package with |
Any updates on this PR? It would be great if it's merged in. |
@abhilashchowdhary Currently working on it 🙂 |
b784e24
to
d5487ab
Compare
d5487ab
to
f18b9c9
Compare
@richafrank could you take a look at this? (forgot to ping you here) |
ACK. Started looking... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes general sense to me. I had a couple questions, and I'm currently confirming our use of py3.4.
tests/test_examples.py
Outdated
@@ -55,7 +55,7 @@ def init_class_fixtures(cls): | |||
serialization='pickle', | |||
) | |||
|
|||
market_data = ('SPY_benchmark.csv', 'treasury_curves.csv') | |||
market_data = ('^GSPC_benchmark.csv', 'treasury_curves.csv') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we make a global name for the default benchmark symbol, so we don't have to change it in 5 places?
@@ -69,6 +69,8 @@ def run_example(example_name, environ): | |||
mod = EXAMPLE_MODULES[example_name] | |||
|
|||
register_calendar("YAHOO", get_calendar("NYSE"), force=True) | |||
# for when we don't actually have a 'test' bundle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When do we have a 'test' bundle otherwise? Did this work before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we ever actually had a test
bundle, but I don't think this failed before when we switched to Google 🤔
.travis.yml
Outdated
@@ -3,7 +3,6 @@ sudo: false | |||
fast_finish: true | |||
python: | |||
- 2.7 | |||
- 3.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, this was removed because of the missing conda packages for the newer pandas-datareader dependency?
I'm currently assessing the internal value of testing this python version...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freddiev4 Can we keep this row in the matrix for running tests, but not for building conda packages?
@@ -45,17 +42,11 @@ def get_benchmark_returns(symbol, first_date, last_date): | |||
""" | |||
data = pd_reader.DataReader( | |||
symbol, | |||
'google', | |||
'yahoo', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the newer version works ok with yahoo again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes on my machine with a fresh zipline install it works.
@freddiev4 Will this PR include fixing the |
@richafrank I don't think it's currently worth adding back Yahoo bundles, which we would need for the Maybe at some other point in time. |
Closing in favor of #2031 |
Google limited their API to only 251 days worth of data, most likely as a result of the change that Yahoo had made a few months ago.
pandas-datareader
made a fix for the Yahoo data reader so I'm making this PR to move back to Yahoo until we can do something better. Ideally we'll have a more robust solution before we do another release, so users will have to install latest Zipline master if this is merged.Hoping to use this as a temp fix for #1965 and all the other issues that reference it
Ran:
zipline run -f buyapple.py -s 2005-1-1 -e 2017-2-1
Output:
Also attempting to drop Py3.4 packages here so that we don't have to build our own
pandas-datareader
andrequests-ftp
packages on conda (also helps make room for Travis jobs when we begin to build Py3.6 packages) #