8
\$\begingroup\$

I am new to Python and here is my first attempt at trying to achieve anything by myself!

Essentially, I am making JSON objects with any user defined ticker through the Alpha Vantage API. It is basically a small part of a Python wrapper for the API (I guess). Actually, someone already made a wrapper for the API but I want to do one for myself. Anyways, I feel like it is a bit more messy than it has to be, and I do not know how to get to the point where someone can type something like object.lastday and get the last day's trading data with the object they just made.

NOTE: not the real API key

Any advice appreciated, I am trying to get better.

import json
import urllib.request
import datetime

#mainURL that will be appended to return our requests
mainURL = "https://www.alphavantage.co/query?"
#API key that will be needed to authenticate
myKey = "F3DH8T01FS5445IB"

#For daily activity
'''
REQUEST NEEDS:
function: will ALWAYS be equal to 'TIME_SERIES_DAILY' for daily
symbol: user defined stock ticker
outputsize: default is 'compact' which only returns 100 data points, 
otherwise
we can define it as 'full' to get up to 20 years worth
datatype: default is 'json', but we can also request 'csv'
'''

#loads a json object of based on what you input in dailyData(SYMBOL)
requestTypeURL = "function=TIME_SERIES_DAILY"
def dailyData(symbol, requestType=requestTypeURL, apiKey=myKey):
    symbolURL = "symbol=" + str(symbol)
    apiURL = "apikey=" + myKey
    completeURL = mainURL + requestType + '&' + symbolURL + '&' + apiURL
    with urllib.request.urlopen(completeURL) as req:
        data = json.load(req)
        return data

#making a json object for Apple and example of getting a date's activity 
(test)
'''
apple = dailyData('AAPL')
print(apple["Time Series (Daily)"]["2018-03-02"])
'''
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Are you using Python 2.x or 3.x? Note for the future we have a tag for each one, it's good to add it to posts for clarity. \$\endgroup\$ – Phrancis Mar 3 '18 at 22:39
  • \$\begingroup\$ @Phrancis I am using 3.6... Thanks for the warm welcome! \$\endgroup\$ – MattMerc Mar 3 '18 at 22:47
  • 1
    \$\begingroup\$ Have you read the official style guide ? I believe Python encourages the use of snake_case for naming. \$\endgroup\$ – yuri Mar 3 '18 at 22:53
8
\$\begingroup\$

I'm going to give you a review of this as to how I'd improve this, and though it pulls some data from the other answer a little bit, it uses a bit different syntax. Namely, I don't like using in-built f"" for formatting strings with replacements, so I use the full "".format() syntax.

Some of the changes are suggested in order to allow easy expansion of this 'API wrapper', so that you can build out new functions more easily. Sorry about that, but I'm one of those coders who has written entire API wrappers and therefore I think in terms of "How could I expand the API" and such.

Also, my apologies for a wall-of-text, but I am overly descriptive sometimes. I also give you the fully revised code at the bottom of this post, if you don't want to read the wall of text, however I suggest that you at least read the section headers in bold, and then the code.


Variables should be snake_case, or CAPS_CASE for 'constants' if you wish.

All variables should be in snake_case. This technically includes the 'global' variables that are essentially constants, though I use CAPS_CASE as they are global constants that won't change (and this is technically acceptable in the PEP8 style guides).

So, all of your variables inside the functions, where applicable, are converted to snake_case, and the URL and API key variables became CAPS_CASE like so:

QUERY_URL = "https://www.alphavantage.co/query?function={REQUEST_TYPE}&apikey={KEY}&symbol={SYMBOL}"
API_KEY = "AN ACTUAL KEY"

Condense URL formatting into a single URL generation method

Your method of generating a URL is clunky. We can glean the following information from your URL structure, though:

  1. You need to provide the type of request, which is a specific type of data from their API.
  2. You need to provide the symbol being looked at.
  3. You need to provide an API key.

Since all three parts are necessary, we can use a structured string to create the URL. We also, already, know the base URL, so we don't need to declare another variable for it. We end up with this string for the QUERY_URL variable (again, I made this one all-caps to be a "global" or a "constant" - this is acceptable, and though it's not snake-case, it's actually acceptable):

QUERY_URL = "https://www.alphavantage.co/query?function={REQUEST_TYPE}&apikey={KEY}&symbol={SYMBOL}"

Similarly, we also have API key as API_KEY:

API_KEY = "AN ACTUAL KEY"

We'll be using this in the next session, so let's focus on that.


Make a generic 'request' function that will accept arguments and issue the request

Rather than structuring the URL and creating individual requests for each type of function you want (which will require code duplication and increase the difficulty of maintaining things down the line), we can take the entire request URL generation and the actual task of making a request and split it off into a separate function.

However, we have to be careful as to how we return the web request. The way your existing code works returns an HTTPRequest object, and not the results of the request. To get the data, you need to read() the data. The data returned from the request is in the form of bytes, and currently the json library is looking for a file pointer (for load) or a string (for loads, which is what you'll want to use). So, we've not only got to do a read() call to the request object, we also have to decode the bytes response into a string with .decode('UTF-8'), as the bytes returned from the API are UTF-8 encoded.

For reasons, I use internal functions that shouldn't be referenced from outside the Python script/library itself which start with a _, to imply that it's a protected member. While there is no mechanism to actually do protected members, most code analysis tools will assume things with underscores are protected elements.

With all this in mind, you get this for your actual web request function, which returns the data, and gets rid of having to take the API key as an argument too:

def _request(symbol, req_type):
    with urllib.request.urlopen(QUERY_URL.format(REQUEST_TYPE=req_type, KEY=API_KEY, SYMBOL=symbol)) as req:
        data = req.read().decode("UTF-8")
    return data

The reasons for doing this become more prevalent in the next section of this review, though.


Change your dailyData function to snake_case, but take advantage of the new _request function

Now that we have the dedicated _request function, we can get rid of all of your URL construction code that you had in dailyData. We can also reduce this function, now to be named get_daily_data, to only require you to pass in the symbol and lets URL construction take place in the actual request handling function we made. This saves us from having to construct brand new URLs within each and every 'getter' that would get data from the API.

All this said and done, you end up with this for your get_daily_data function, which looks a bit cleaner than the original version, in my opinion:

def get_daily_data(symbol):
    return json.loads(_request(symbol, 'TIME_SERIES_DAILY'))

Now, the only thing you really need to do is make sure you're passing the proper request/function string to the _request library, and also pass the symbol.

This has an added bonus, though: because we're using _request to do the URL construction and the actual web request processing, we can easily expand this code for other functions in your API wrapper - all you need to change is the 'function' call to get different data.


Unused Libraries

You import datetime but you don't use it for anything. So, don't include it in the import statements until you actually use it for something.


Now on to testing. I use a slightly different form of your test code here, compressing everything into a one-liner call:

print(get_daily_data("AAPL")["Time Series (Daily)"]["2018-03-02"])

This in turn returns this data:

{'1. open': '172.8000', '2. high': '176.3000', '3. low': '172.4500', '4. close': '176.2100', '5. volume': '38453950'}

The code after all my revisions above, and after removing comments, ends up being this:

import json
import urllib.request

QUERY_URL = "https://www.alphavantage.co/query?function={REQUEST_TYPE}&apikey={KEY}&symbol={SYMBOL}"
API_KEY = "AN ACTUAL KEY"

def _request(symbol, req_type):
    with urllib.request.urlopen(QUERY_URL.format(REQUEST_TYPE=req_type, KEY=API_KEY, SYMBOL=symbol)) as req:
        data = req.read().decode("UTF-8")
    return data

def get_daily_data(symbol):
    return json.loads(_request(symbol, 'TIME_SERIES_DAILY'))

This being said, this program could be developed to take the stock ticker to check as a runtime argument, thereby permitting you to run the core code against any symbol that you define, but that's outside the scope of this review, technically.

\$\endgroup\$
  • \$\begingroup\$ Wow! This is incredible. I like how simplistic it is and I do like the detail and explanation about possible expansion. Thanks a million! \$\endgroup\$ – MattMerc Apr 4 '18 at 20:34
5
\$\begingroup\$

I will drop you an updated version of the code, it basically changes small things

  • Renamed variables to follow snake_case convention
  • Make use of Python 3 f"" function to create strings
  • Restructured code for creating the url
  • Added main function to run script directly

    def get_daily_data(symbol, request_type, api_key):
    
        url = f"{main_url}{request_type}&symbol={symbol}&apikey={api_key}"
    
        with urllib.request.urlopen(url) as req:
            return json.load(req)
    

    The main part will look like this

    if __name__ == "__main__":
        # Test: making a json object for Apple and example of getting a date's activity
        request_type_url = "function=TIME_SERIES_DAILY"
    
        apple = get_daily_data('AAPL', request_type=request_type_url, api_key=my_key)
        print(apple["Time Series (Daily)"]["2018-03-02"])
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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