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: Add support for AlphaVantage API #490

Merged
merged 7 commits into from
Apr 13, 2018

Conversation

addisonlynch
Copy link
Contributor

@addisonlynch addisonlynch commented Feb 19, 2018

AlphaVantage Readers

Adds support for nearly all of the AlphaVantage API endpoints.

Additions

  • AVTimeSeriesReader - class to handle requests for data from the Stock Time Series endpoints
    • top-level get_data_av, for TIME_SERIES_DAILY
    • av-daily for TIME_SERIES_DAILY
    • av-daily-adjusted for TIME_SERIES_DAILY_ADJUSTED
    • av-weekly for TIME_SERIES_WEEKLY
    • av-weekly-adjusted for TIME_SERIES_WEEKLY_ADJUSTED
    • av-monthly for TIME_SERIES_MONTHLY
    • av-monthly-adjusted TIME_SERIES_MONTHLY_ADJUSTED
  • AVQuotesReader - class to handle requests for Batch Stock Quotes using top-level get_quote_av
  • AVForexReader - class to handle FX Exchange Rates endpoint (av-forex or top-level get_exchange_rate_av)
  • AVSectorPerformanceReader - class to handle Sector Performances SECTOR endpoint av-sector or top-level get_sector_performance_av
  • AlphaVantage- base class for handling AlphaVantage requests

Questions

  • What format should the sector performances return? As of now, they're strings with a percent sign at the end. Using int64 and float64 for the other new readers.
  • Is it okay for TimeSeriesReader to take the argument function which allows the selection of the daily/monthly/weekly endpoint? I have an implementation (see this gist) which uses separate readers for each (AVDailyReader, AVDailyAdjustedReader, etc.). Can switch over to that if it is more in-line with the current style.
  • Used AV instead of AlphaVantage in names...readers such as AlphaVantageTimeSeriesReader seemed a bit long.

Needs Help

  • Will need to obtain an API key from AlphaVantage and contact them (support@alphavantage.co) to request an exemption from throttling to allow the tests to run. Seems they don't like repeated calls to the same endpoints without additional permission. As such, the tests now will pass with an API key that does not have these restrictions, but may fail otherwise.
    • This key will be stored as the environment variable ALPHAVANTAGE_API_KEY

Near Future

The above takes care of most of the AlphaVantage endpoints. There are two groups remaining:

  1. Digital/Crypto Currencies - is this something we would want to implement? I don't know if this is a direction you guys would want to head in for PDR. I don't have a lot of knowledge in this sphere, but it seems as if these endpoints could be easily implemented if desired.
  2. Technical Indicators - what is the preferred way to implement this?
    AlphaVantage provides 52 technical indicators, each of which seems to take the same default (required) parameters: function, symbol, interval, apikey. However, certain indicators (such as EMA) require additional parameters time_period, series_type, etc.

In addition, will need to add a reader to handle TIME_SERIES_INTRADAY

@gliptak
Copy link
Contributor

gliptak commented Feb 19, 2018

@addisonlynch Please review the Travis failures

https://travis-ci.org/pydata/pandas-datareader/builds/343250680

While most of them API key related, this ones fails on import pandas.testing

https://travis-ci.org/pydata/pandas-datareader/jobs/343250681

@davidastephens @jreback Who can add the AlphaVantage key to Travis and ReadTheDocs?

Thanks

@bashtage
Copy link
Contributor

Need to use same test pattern to skip module if API key is not available.

@bashtage
Copy link
Contributor

@gliptak I can add a key to Travis. It won't actually be used for testing until the PR is merged since PRs can't access encryption variables to prevent leaking them by someone who could submit a useless PR that had print(os.environ.get['SEKRIT'])

@addisonlynch
Copy link
Contributor Author

addisonlynch commented Feb 20, 2018

I added the pytest.skipifs and fixed pandas 0.19.2 compat. Do you want me to add @skip_on_exception(RemoteDataError) to the tests as well? Certain tests will still fail unexpectedly because of:

{'Information': 'Thank you for using the CURRENCY_EXCHANGE_RATE_API! Please kindly contact support@alphavantage.co if you would like to have a higher API call volume'}

unless PDR does obtain a higher call volume key. Let me know what works best.

Copy link
Contributor

@bashtage bashtage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. Some small changes and doc string fixes. Also, could you add to readers/index.rst

access_key=os.getenv('ALPHAVANTAGE_API_KEY'))
f.loc["2017-02-09"]

The top-level function ``get_data_av`` is also provided. This function will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like get_data_av -- how could a new user know what av is? Maybe just the long get_data_alphavantage? Autocomplete is pretty good.


class AlphaVantage(_BaseReader):
"""
Base class for all AlphaVantage queries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be docstring here. Maybe we should use a doc string inheriter to simplify this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind - this is a base class.


Parameters
----------
pairs : string, array-like object (list, tuple, Series)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just keep the standard symbols?


Parameters
----------
retry_count : int, default 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this docstring has all the arguments from the base class.

@@ -47,6 +51,10 @@
'DataReader']


def get_data_av(*args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_data_alphavantage?

@addisonlynch
Copy link
Contributor Author

Done

@bashtage bashtage added this to the 0.7.0 milestone Feb 21, 2018
@addisonlynch
Copy link
Contributor Author

addisonlynch commented Feb 21, 2018

@bashtage was going to add the remaining endpoints today or tomorrow. Do you want me to push them to this pull or a separate one? Also, any direction you're able to provide on the preferred way to implement the technical indicators (see above) would be great. Thanks

@gliptak
Copy link
Contributor

gliptak commented Mar 3, 2018

@bashtage Were your review items addressed?

@addisonlynch Could you resolve the conflict?

Thanks

@addisonlynch
Copy link
Contributor Author

@gliptak conflicts resolved. Did indeed repair the requested changes

@addisonlynch
Copy link
Contributor Author

Travis fails on unrelated errors (FRED and tiingo?). Are we ready to merge this otherwise?

@gliptak
Copy link
Contributor

gliptak commented Apr 4, 2018

@bashtage Did you complete your review? Thanks

@bashtage bashtage merged commit 1763dbd into pydata:master Apr 13, 2018
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

Successfully merging this pull request may close these issues.

Add Alpha Vantage as data source?
3 participants