2
\$\begingroup\$

Currently my code works but I wanted to know if I'm going about this the right way. I want to be able to individually call variables from method 'data_by_name' in 'otherfile.py' . At the moment I import 'otherfile.py' and put the elements into a list within 'main.py' and use tuple unpacking to give each element a variable name.

Could I go about this a better way?

otherfile.py

import requests
import json

API_KEY = "XXX"
PROJECT_TOKEN = "XXX"
RUN_TOKEN = "XXX"

class Data:
    def __init__(self, api_key, project_token):
        self.api_key = api_key
        self.project_token = project_token
        self.params = {"api_key":api_key}
        self.get_data()

    def get_data(self):
        r = requests.get('https://www.parsehub.com/api/v2/projects/PROJECT_TOKEN/last_ready_run/data', params={"api_key": API_KEY})
        self.data = json.loads(r.text)


    def data_by_name(self,country):

        data = self.data['country']
        for content in data:
            if content['Cname'].lower() == country.lower():
                print(content)
                name = content['Cname']
                pop = content['population']
                popRank = content['popRank']
                growth = content['growthRate']
                per = content['worldPer']
                area = content['area']
                capital = content['capital']
                region = content['region']
                gpd = content['gpd']
        return(name,pop,popRank,growth,per,area,capital,region,gpd)

if __name__ == '__main__':
    data = Data(API_KEY,PROJECT_TOKEN)
    data.data_by_name('Russia')

main.py

from ParsehubCode import Data,API_KEY,PROJECT_TOKEN

country = Data(API_KEY,PROJECT_TOKEN)
test = country.data_by_name("china")
list1 = []
for element in test:
    list1.append(element)
for x in list1:
    A,B,C,D,E,F,G,H,I = list1
print(A,B,C)

country.json

{
 "country": [
  {
   "Cname": "China",
   "name": "China Population 2020 (Live)",
   "population": "1,438,719,354",
   "popRank": "1",
   "growthRate": "0.39%",
   "worldPer": "18.47%",
   "area": "9,706,961 km²",
   "capital": "Beijing",
   "region": "Asia",
   "gpd": "$10,746.78"
  }
{
\$\endgroup\$
3
\$\begingroup\$

Instead of

list1 = []
for element in test:
    list1.append(element)
for x in list1:
    A,B,C,D,E,F,G,H,I = list1

you could simply write

A,B,C,D,E,F,G,H,I = test

Regardless, here's a slightly different approach.

Assuming that the input json structure is { 'country': [{ }, ..., { }] }, I'd slightly modify self.data:

 self.data = {country['Cname'].lower(): country
              for country in json.loads(r.text)['country']}

then access this dictionary whenever required. Possibly by overwriting __getitem__:

def __getitem__(self, key):
    return self.data[key.lower()]

Then you can access the inner dictionary using [ ]:

GlobalDataLoader(API_KEY,PROJECT_TOKEN)['china']
GlobalDataLoader(API_KEY,PROJECT_TOKEN)['china']['population']

Here I've renamed the Data class: I don't expect the constructor of a class named Data to access the internet, and the data seems to be global. Still, I'd remove the self.get_data() call from __init__.

On the other hand, if you are not planning to reuse the (API_KEY, PROJECT_TOKEN) for a specific GlobalDataLoader instance, then I wouldn't store these in the instances. Instead do something like

data = GlobalDataLoader(project_secrets)

where

class GlobalDataLoader:    
    def __init__(self, project_secrets):
        self.load_data(project_secrets)

    def load_data(self, project_secrets):
        url = f"https://www.parsehub.com/api/v2/projects/{project_secrets['project_token']}/last_ready_run/data"
        r = requests.get(url, params={"api_key": project_secrets['api_key']})
        self.data = {country['Cname'].lower(): country
                     for country in json.loads(r.text)['country']}

    def __getitem__(self, key):
        return self.data[key.lower()]

That being said, don't hard-code secrets into the source code.

API_KEY = "XXX"
PROJECT_TOKEN = "XXX"
RUN_TOKEN = "XXX"

You don't want this as a habit when you start using version control systems. Store these in a separate file. If you have to store that in a VCS, then encrypt it (e.g. use gitcrypt).

To reuse the idea of a wrapped dictionary, you could write a

class ProjectSecretsLoader:
    def __init__(self, config_file_path):
        self.load_config(config_file_path)

    def load_config(self, config_file_path):
        self.config_data = dict() # TODO

    def __getitem__(self, key):
        return self.config_data[key]

then do

config_file_path = "..."
project_secrets = ProjectSecretsLoader(config_file_path)
data = GlobalDataLoader(project_secrets)

Last, but not least requests.get might fail, json.loads might fail. The result of json.loads might not have the structure you expect, or might be missing the country you will eventually try to look up, etc. In these cases specific exceptions will be raised, which ideally should be handled, otherwise the program will crash.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ It's somewhat common to have secrets in a secrets.py this way you can just import secrets and then do GlobalDataLoader(secrets). \$\endgroup\$ – Peilonrayz May 22 at 12:46
2
\$\begingroup\$

Since you download the data about all countries from the API, why not save it such that getting the countries by name is easy and fast? Like a dictionary with content['Cname'].lower() as key?

class Data:
    keys = ['Cname', 'population', 'popRank', 'growthRate', 'worldPer', 'area',
            'capital', 'region', 'gdp']

    def __init__(self, api_key, project_token):
        self.url = f'https://www.parsehub.com/api/v2/projects/{project_token}/last_ready_run/data'
        self.params = {"api_key": api_key}
        self.get_data()

    def get_data(self):
        r = requests.get(self.url, params=self.params)
        r.raise_for_status()
        self.data = r.json()
        self.countries = {item['Cname'].lower():
                          {key: item[key] for key in self.keys}
                          for item in self.data}

    def data_by_name(self,country):
        return self.countries(country.lower())

Note that I did not replicate your returning of a tuple, but just returned the dict.If you do need them in local variables, you have multiple options:

  1. Just use test["gdp"] instead of I.
  2. Use a collections.namedtuple as return value and do test.gdp.
  3. Use locals().update(test) and the afterwards just gdp.

The first two I would prefer over the third and over your code.

Note that A,B,C,D,E,F,G,H,I are really bad names. For one, they don't follow Python's official style-guide, PEP8, which recommends lower_case for variables and functions. It also does note make it clear what each variable contains.


This

test = country.data_by_name("china")
list1 = []
for element in test:
    list1.append(element)
for x in list1:
    A,B,C,D,E,F,G,H,I = list1

can also be shortened to

A,B,C,D,E,F,G,H,I = country.data_by_name("china")
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$
  1. self.data is a bad way to cache data. There are two ways to address this.

  2. We can use a session object to remove the need for passing params in get_data. It also gives some other nice benefits, but we're not really using them here.

  3. data_by_name doesn't belong on Data. This should be created and stored outside the class.

parsehub.py

import functools

import requests


class Parsehub:
    def __init__(self, api_key, project_token):
        self._session = s = requests.Session()
        s.params.update({'api_key': api_key})
        self._project_token = project_token

    @functools.lru_cache
    def get(self, path):
        return self._session.get(
            f'https://www.parsehub.com/api/v2/projects/'
            f'{self._project_token}'
            f'/{path}'
        )

    def get_data(self):
        # Code to format the countries to your desired format goes here
        return self.get('last_ready_run/data').json()

__main__.py

from .settings import API_KEY, PROJECT_TOKEN
from .parsehub import Parsehub


def main():
    parse_hub = Parsehub(API_KEY, PROJECT_TOKEN)
    # The other answers have shown how to get this in the format you want.
    countries = {
        country['Cname'].lower(): country
        for country in parse_hub.get_data()
    }
    country = countries['russia']


if __name__ == '__main__':
    main()
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You are missing the json conversion, probably in get_data. Otherwise, I agree, this is more maintainable. \$\endgroup\$ – Graipher May 22 at 14:07
  • 1
    \$\begingroup\$ @Graipher Aw shucks, how'd I miss that! Thank you :) \$\endgroup\$ – Peilonrayz May 22 at 14:09

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.