3
\$\begingroup\$

I've been working on a little challenge in order to learn more.. I've written the following code, which works. I'm just concerned that there are several areas where my lack of in depth Python knowledge has forced me to 'go the long way around'. I'm really keep to learn more and learn in the correct way, so looking for areas I can improve the code & also shorten it where possible.

The idea is, we use a free API to obtain some JSON data containing a list of dates and stock price information. The JSON gives a 'refreshed date' which is the latest date the data has been obtained, I'm then calculating a list of N dates PRIOR to this date, and returning the stock close price for each of those past days, and then returning the average of those prices.

The code works, and I'm quite happy of that.. but I don't want to stop here. I want to make sure I'm learning the right way.

Since the stock market is closed on Sat/Sun, we need to avoid weekends when calculating the list of dates, so N=3 on a Monday, would be 3 'stock market' days prior to Monday, thus - Mon, Fri, Thu.

For anyone who is interested in looking at the format of data being read in there's a demo API key: https://www.alphavantage.co/query?apikey=demo&function=TIME_SERIES_DAILY_ADJUSTED&symbol=MSFT

CODE:

from datetime import date, timedelta, datetime
import json
from requests import Request, Session
from flask import Flask, make_response

app = Flask(__name__)

# To be passed in with ENV-Vars
SYMBOL='MSFT'
NDAYS = 3

api_key = 'apikey="XXXXXXXX"&'
function = 'function=TIME_SERIES_DAILY_ADJUSTED&'
symbol = f'symbol={SYMBOL}'
url = f'https://www.alphavantage.co/query?{api_key}{function}{symbol}'

session = Session()
output = session.get(url)
data = json.loads(output.text)

refreshed = datetime.strptime(str(data['Meta Data']['3. Last Refreshed']), '%Y-%m-%d').date()
dates = []
output = {}
def prev_days(rdate):
    rdate -= timedelta(days=NDAYS)
    while rdate.weekday() > 4:
        rdate -= timedelta(days=1)
    return rdate

past_date = prev_days(refreshed)

delta = refreshed - past_date
for i in range(delta.days + 1):
    dates.append(refreshed - timedelta(days=i))

for date in dates:
    close = data['Time Series (Daily)'][str(date)]['4. close']
    output.update({str(date): float(close)})

avg = sum(list(output.values())) / len(list(output.values()))

def resp():
    return f'{SYMBOL} data={list(output.values())}, average={avg}'

@app.route('/')
def main():
    response = make_response(resp(), 200)
    response.mimetype = "text/plain"
    return response

if __name__ == "__main__":
    app.run(host='0.0.0.0', port=5000)
\$\endgroup\$
1
  • 3
    \$\begingroup\$ Whoever thought of "1. open" as response structure is definitely not developer friendly. \$\endgroup\$
    – hjpotter92
    Nov 2, 2020 at 11:09

1 Answer 1

2
\$\begingroup\$

Relays

Your current design:

  • fetches data from alphavantage once, on startup
  • offers it to anyone who hits any of your interfaces on port 5000 (which, by the way, is already used by a large number of services other than the default Flask development webserver)

This seems unusual. Perhaps it's just to try out a random use of Flask for learning purposes, which is fine, but:

If you really want to preserve this API as an effective relay, it should probably be authenticated. You can add that here, or in a frontend (nginx, etc). Also, just bind to 80 once you hit production. A frontend can do HTTPS termination effectively translating from 443 to 80.

If all you want is code to conveniently get alphavantage data, don't make your own HTTP API; just make a requests wrapper library.

Globals

To be passed in with ENV-Vars

should apply to api_key, as that's a secret that should not be hard-coded.

Also, it's an odd choice to hard-code symbol. With very little introduced complexity, you can make your code parametric on symbol (and perhaps also function).

Otherwise: session, output, data, etc. probably shouldn't be globals. Consider making an LRU cache for data, and taking advantage of Flask's before_first_request, and moving the great majority of your global code into functions.

For session in particular, there's currently no advantage to having that - you might as well just requests.get since you only do it once.

Request formation

Don't pre-format key-value query parameter pairs. Instead,

session.get(
    'https://www.alphavantage.co/query',
    params={
        'apikey': API_KEY,
        'function': 'TIME_SERIES_DAILY_ADJUSTED',
        'symbol': 'MSFT',
    },
)

Response parsing

json.loads(output.text)

is unnecessary. Just use output.json. Also, between get() and .json, call output.raise_for_status. This will increase the quality of error information when something goes wrong.

Sum over an iterable

sum(list(output.values()))

should not use list. sum can operate on any iterable.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.