quantopian / empyrical

Common financial risk and performance metrics. Used by zipline and pyfolio.
https://quantopian.github.io/empyrical
Apache License 2.0
1.31k stars 406 forks source link

DEP: Deprecate all data reading functionality via pandas-datareader #97

Closed eigenfoo closed 6 years ago

eigenfoo commented 6 years ago

Closes #96 #89 #65 #64

Deprecates all data-fetching functionality via pandas-datareader, and adds a simple_returns helper function to replace that functionality that was implicitly done in the data-fetchers.

To quote the REAME:

As of early 2018, Yahoo Finance has suffered major API breaks with no stable replacement, and the Google Finance API has not been stable since late 2017 (source). In recent months it has become a greater and greater strain on the empyrical development team to maintain support for fetching data through pandas-datareader and other third-party libraries, as these APIs are known to be unstable.

As a result, all empyrical support for data reading functionality has been deprecated and will be removed in a future version.

Users should beware that the following functions are now deprecated:

Users should expect regular failures from the following functions, pending patches to the Yahoo or Google Finance API:

eigenfoo commented 6 years ago

@twiecki @richafrank requesting review from research and engineering, ready to merge otherwise.

twiecki commented 6 years ago

LGTM. One thing we should add, though, is a function that takes prices and computes correctly indexed returns which one can pass into pyfolio.

eigenfoo commented 6 years ago

@twiecki done

twiecki commented 6 years ago

Thanks, this definitely requires engineering review. CC @richafrank

twiecki commented 6 years ago

That sounds like a good solution to me.

On Fri, Jun 1, 2018 at 4:39 PM, George Ho notifications@github.com wrote:

@eigenfoo commented on this pull request.

In empyrical/utils.py https://github.com/quantopian/empyrical/pull/97#discussion_r192417144:

@@ -24,6 +24,16 @@ import pandas as pd from pandas.tseries.offsets import BDay from pandas_datareader import data as web

@twiecki https://github.com/twiecki @richafrank https://github.com/richafrank there is a known import error with pandas_datareader: it has been fixed upstream here https://github.com/pydata/pandas-datareader/pull/520, but has yet to be released in a new version. See this StackOverflow thread https://stackoverflow.com/questions/50394873/import-pandas-datareader-gives-importerror-cannot-import-name-is-list-like .

Wondering what we should do about this. Perhaps wrap it in a try-except, and raise a warning if the import fails? If we're deprecating pandas-datareader functionality anyways, it doesn't make sense to keep this line around if it raises an import error.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/quantopian/empyrical/pull/97#pullrequestreview-125194322, or mute the thread https://github.com/notifications/unsubscribe-auth/AApJmHDvFXNEj4bVyfBRWfVs4zWxUQnrks5t4VIkgaJpZM4UFLcn .

eigenfoo commented 6 years ago

Done.

eigenfoo commented 6 years ago

Note to self: still need to write a test for the simple_returns function.

eigenfoo commented 6 years ago

Not quite sure why tests are failing. The checks fail on my local machine as well, but using pdb doesn't reproduce the error.

Any pointers @ssanderson @twiecki?

eigenfoo commented 6 years ago

@twiecki fixed unit tests. Ready for another pass, or merging.

ssanderson commented 6 years ago

@eigenfoo I had one more comment on the README. Otherwise this LGTM.

twiecki commented 6 years ago

Thanks @eigenfoo!