8
\$\begingroup\$

I would really appreciate if someone could take a look at this code. I am not very advanced, so ANY kind of feedback and critique would be precious to me.

What this program is supposed to do is to go to a given URL, download the list of cities, for which it will download the data, based on a https://openweathermap.org/api . It should be run on a regular basis and write all of the results to the CSV file.

It is divided into two parts. The first one consists only of the scheduler, final function which is being run by it and the list of columns I want to get as the final result.

Code is written for Python 3.6.

from API.helpers import get_weather_data, json_to_df, create_dict
import schedule, time

URL = 'https://pm1aapplicantsdata.blob.core.windows.net/databases/CitiesWeather/CitiesWeather.csv'
columns = ["name","sys.country","main.temp",
           "main.humidity","main.pressure",
           "visibility", "wind.speed"]
#Writing results to CSV
def weather_api(URL):
    dict = create_dict(URL)
    for city, code in dict.items():
        data = get_weather_data(city, code)
        json_to_df(data, columns)
schedule.every().day.at("10:30").do(weather_api, URL)
while True:
    schedule.run_pending()
    time.sleep(1)

Here is the second part, which is my "helper" file.

import json
import requests
from pandas.io.json import json_normalize
import pandas as pd
import os
import requests
import csv
api_key = "xxxxxxxxxxxxxxxxxxxxxxx"

#function to build api requests
def build_request(city, code):
    base_url = "http://api.openweathermap.org/data/2.5/weather?"
    complete_url = base_url + "appid=" + api_key + "&q=" + city +"," + code
    return complete_url

#function to get weather data
def get_weather_data(city, code):
    url = build_request(city, code)
    try:
        response = requests.get(url)
        response.status_code
    except requests.exceptions.HTTPError:
        print('Error occured while downloading data')
    except requests.exceptions.URLError:
        print('Error occured while downloading data')
    citydataJSON = response.text
    citydata = json.loads(citydataJSON)
    return citydata

def json_to_df(data, columns):
    df = pd.DataFrame.from_dict(json_normalize(data), orient='columns')
    new_df = df[columns]
    new_df.insert(0, 'TimeStamp', pd.datetime.now().replace(microsecond=0))
    if not os.path.isfile('weather.csv'):
        return new_df.to_csv('weather.csv', header='column_names', index=False)
    else:
        return new_df.to_csv('weather.csv', mode='a', header=False, index=False)
#creating a dictionary of cities and city codes(based on the CSV file downloaded from a given URL
def create_dict(URL):
    with requests.Session() as s:
        dict = {}
        download = s.get(URL)
        decoded_content = download.content.decode('utf-8')
        cs = csv.reader(decoded_content.splitlines(), delimiter=',')
        next(cs, None)
        my_list = list(cs)
        for row in my_list:
            dict[row[0]] = row[1]
        return dict
\$\endgroup\$
  • \$\begingroup\$ What Python version did you write this for? \$\endgroup\$ – Mast Sep 28 '18 at 11:45
  • 1
    \$\begingroup\$ @Graipher Oops, my misunderstanding. \$\endgroup\$ – 200_success Sep 28 '18 at 15:13
4
\$\begingroup\$

Your code is very nicely split into functions. As a next step I would split it to classes. What you want to do is

  1. Collect the data for a given city
  2. Convert it into a schema that fits your purposes and write it to a file

In my opinion, the scheduling of this job should be done elsewhere, e.g. cron

Although the +-sign concatenates strings correctly it is not the most readable way to do it. There are many ways to format strings in Python3 and I tend to use f-strings.

Pandas is a great library for data analysis and data "wrangling" but to use it to write csv files is simply an overkill. Usually, you want to keep your virtual environmets as small as possible and use the standard library as much as possible. In this case the csv library is all you need.

I think you got somehow lost when modifying the API response. There is absolutely no need to convert it from dictionary to json and then back to tabular form. Simply use the dictionary that requests gives you, get the data you need and write it to a file.

Here's my code:

from os.path import isfile
from io import StringIO
from datetime import datetime
from csv import DictWriter
import requests


URL = 'https://pm1aapplicantsdata.blob.core.windows.net/databases/CitiesWeather/CitiesWeather.csv'

class CityWeatherCollector(object):
    """Class to collect weather information for a given city from openweathermap.org
    """

    base_url = "http://api.openweathermap.org/data/2.5/weather?"
    apikey = 'xxxxxxxxxxxxxxxxxxxxxxx'

    def __init__(self, city, code):
        self.city = city
        self.code = code

    @property
    def full_url(self):
        return self.base_url + f"appid={self.apikey}&q={self.city},{self.code}"

    def fetch_data(self):
        response = requests.get(self.full_url)
        if response.status_code == 200:
            #returns a dictionary
            return response.json()
        else:
            raise WeatherCollectionError(f"Response from API was: {response.status_code}")


class WeatherCollectionError(Exception):
    pass


class WeatherDataWriter(object):

    def __init__(self, full_filepath):
        self.full_filepath = full_filepath

class WeatherData(object):
    """Class for a representation of the data"""

    def __init__(self, name, country, temp, humidity, pressure, visibility, wind_speed, timestamp=datetime.now()):
        self.name = name
        self.country = country
        self.temp = temp
        self.humidity = humidity
        self.pressure = pressure
        self.visibility = visibility
        self.wind_speed = wind_speed
        self.timestamp = timestamp

    @staticmethod
    def create_from_json(json_dict, timestamp=datetime.now()):
        return WeatherData(
            name=json_dict['name'],
            country=json_dict['sys']['country'],
            temp=json_dict['main']['temp'],
            humidity=json_dict['main']['humidity'],
            pressure=json_dict['main']['pressure'],
            visibility=json_dict['visibility'],
            wind_speed=json_dict['wind']['speed'],
            timestamp=timestamp
        )

    def write_one(self, outfile):
        weather_data = self.__dict__
        # if file exists, append
        csv_writer = DictWriter(
            out_file,
            fieldnames=list(weather_data.keys()),
            delimiter=',',
            lineterminator='\n'
            )
        #if the file is empty write header
        if outfile.tell() == 0:
            csv_writer.writeheader()
        csv_writer.writerow(weather_data)


def get_cities(url=URL):
    response = requests.get(url)
    if response.status_code == 200:
        decoded_response = StringIO(response.content.decode('utf8'))
        # pop the headings
        next(decoded_response)
        for line in decoded_response:
            city, code = tuple(line.strip().split(',')) 
            yield CityWeatherCollector(city, code)


if __name__ == '__main__':
    timestamp = datetime.now()
    with open('data.csv', 'a') as out_file:
        for collector in get_cities(URL):
            # add error handling
            full_data = collector.fetch_data()
            data = WeatherData.create_from_json(json_dict=full_data, timestamp=timestamp)
            data.write_one(out_file)
\$\endgroup\$
2
\$\begingroup\$

Nice project! I would bring another point not touched by kaidokuupa:

You should follow PEP8 which is the official style guide for writing Python. It's a set of guideline to write code readable by others, which is very important when you will be working on open source project and/or in an enterprise environment.

To help you conform to this standard, you can use what is called "Linter" which is a software telling you if you follow the standard (or you can configure it to implement your own standard most of the time). Example of linter for Python are flake8, Black (this one also rewrite your code), Pylint and many more I'm not aware of probably.

Using a linter can be integrated with a Continuous Integration tool when you use your favorite CVS (Git, Mercurial, SVN ...)

Another point is your comments should describe the 'Why' not 'What'. For example

#function to get weather data
def get_weather_data(city, code):
    ...

This comment is not very helpful. And to comment function/method you should probably use docstring instead. A good article on writing code comment here by one of the founders of StackOverflow.com.

\$\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.