6
\$\begingroup\$

I've created the following app in Python 3.5. The app uses pyowm and will take a location and return weather information. I've also built a GUI with tkinter for it.

Keen to find out some tips to improve my coding style.

import tkinter as tk
import pyowm
import datetime, time


class WeatherInfo(tk.Tk):

    def __init__(self):
        tk.Tk.__init__(self)
        self.wm_title('Weather')
        self.temp = tk.StringVar(self,value='')
        self.humid = tk.StringVar(self,value='')
        self.status = tk.StringVar(self,value='')
        self.sunrise = tk.StringVar(self,value='')
        self.sunset = tk.StringVar(self,value='')
        self.ld = tk.StringVar(self,value = '')
        self.ln = tk.StringVar(self,value = '')

        self.ask = tk.LabelFrame(self, text ='Location')
        self.ask.pack(fill='both',expand='yes')
        self.kw_label = tk.Label(self.ask, text ='Get weather in:')
        self.kw_label.pack(side = tk.LEFT)
        self.kw = tk.Entry(self.ask)
        self.kw.pack(side = tk.RIGHT)
        self.rb = tk.Button(self, text='Go', command = self.search)
        self.rb.pack()
        self.owm = pyowm.OWM('*CENSORED*')

        self.output = tk.LabelFrame(self, text ='Information')
        self.output.pack(fill='both',expand='yes')
        tk.Label(self.output, textvariable = self.temp).pack()
        tk.Label(self.output, textvariable=self.humid).pack()
        tk.Label(self.output, textvariable=self.status).pack()
        tk.Label(self.output, textvariable=self.sunrise).pack()
        tk.Label(self.output, textvariable=self.sunset).pack()
        tk.Label(self.output, textvariable=self.ld).pack()
        tk.Label(self.output, textvariable=self.ln).pack()
        button = tk.Button(master=self, text='Quit', command=self._quit)
        button.pack(side=tk.BOTTOM)

    def search(self):
        obs = self.owm.weather_at_place(self.kw.get())
        try:
            w = obs.get_weather()
            self.temp.set('Temperature: ' +str(round(w.get_temperature()['temp'] - 273.15,0))+ ' C')
            self.humid.set('Humidity: '+str(w.get_humidity())+ '%')
            self.status.set('Status: ' + w.get_status())
            self.sunrise.set('Sunrise at '+datetime.datetime.fromtimestamp(w.get_sunrise_time()).strftime('%H:%M:%S'))
            self.sunset.set('Sunset at '+datetime.datetime.fromtimestamp(w.get_sunset_time()).strftime('%H:%M:%S'))
            self.ld.set('Day length: '+str(round((w.get_sunset_time() - w.get_sunrise_time()) / 3600,1))+' h')
            self.ln.set('Night length: '+str(round(24 - (w.get_sunset_time() - w.get_sunrise_time()) / 3600,1))+' h')
        except:
            self.temp.set('Pick a city to display weather.')
    def _quit(self):
        self.quit()
        self.destroy()
app = WeatherInfo()
app.mainloop() 
\$\endgroup\$
2
  • \$\begingroup\$ I am getting an exception "urllib.error.HTTPError: HTTP Error 401: Unauthorized" when i type in a location, what format should the location be in? City, State, Country? Latitude, Longitude? \$\endgroup\$ Sep 14 '16 at 0:40
  • \$\begingroup\$ @newToProgramming you need an API key for the self.owm = pyowm.OWM('API KEY HERE'). Concerning the location, just enter whatever you would enter in Google Maps - a city and optionally the state and country \$\endgroup\$
    – cnstlungu
    Sep 14 '16 at 9:47
2
\$\begingroup\$

Your code has a few different responsibilities, most of which are currently in the search function:

  1. Setting up the GUI
  2. Retrieving the weather report
  3. Parsing the relevant data out of that report
  4. Displaying the result

Each of these deserves their own function. I would also abstract the format itself away into a constant of the class. This allows looping over the different keys to build the objects instead of having to do it manually. I used a collections.OrderedDict to preserve the order of them.

These template strings are:

class WeatherInfo(tk.Tk):
    templates = OrderedDict([
        ('temp', 'Temperature: {temp:.1f} C'),
        ('humid', 'Humidity: {humid}%'),
        ('status', 'Status: {status}'),
        ('sunrise', 'Sunrise at {sunrise:%H:%M:%S}'),
        ('sunset', 'Sunset at {sunset:%H:%M:%S}'),
        ('day_length', 'Day length: {day_length:.1f} h'),
        ('night_length', 'Night length: {night_length:.1f} h')])

Note that you can already define the format for the date in there and also the rounding for floats.

This allows doing this in __init__:

def __init__(self):
    ...
    self.tk_info = {key: tk.StringVar(
        self, value='') for key in WeatherInfo.templates}
    ...
    self.labels = []
    for key in WeatherInfo.templates:
        self.labels.append(
            tk.Label(self.output, textvariable=self.tk_info[key]).pack())
    ...

search should just be concerned with getting the result from the web, catching an error if needed. Here you should notice that you should never have a bare except. It will also catch e.g. the user pressing CTRL-C to abort the program. Always use at least except Exception which will catch almost everything (and not some special exceptions like CTRL-C). Here you can be more specific, because the code will throw a AttributeError if unsuccessful:

def search(self):
    obs = self.owm.weather_at_place(self.kw.get())
    try:
        return json.loads(obs.get_weather().to_JSON())
    except AttributeError:
        self.tk_info['temp'].set('Pick a city to display weather.')

Note that I am using the json format here, just because I think it is more intuitive to work with. But you can also use the get_* functions in parse.

Speaking of which: I would add a parse function that takes the result of obs.get_weather and does all the conversions/calculations. It returns a dictionary with the raw values we want to put in the template:

def parse(self, w):
    parsed_weather = {'temp': round(w['temperature']['temp'] - 273.15, 0),
                      'humid': w['humidity'],
                      'status': w['status'],
                      'sunrise': datetime.fromtimestamp(w['sunrise_time']),
                      'sunset': datetime.fromtimestamp(w['sunset_time']),
                      'day_length': round((w['sunset_time'] - w['sunrise_time']) / 3600, 1),
                      'night_length': round(24 - (w['sunset_time'] - w['sunrise_time']) / 3600, 1)}
    return parsed_weather

I also used from datetime import datetime in the header to get rid of one level of redundant naming.

Then I would define a update function, which updates all the labels to the correct wording. Here it comes in handy that we already have the template strings defined and have a dictionary of the values we want to put in there!

def update(self, report):
    for key, template in WeatherInfo.templates.items():
        self.tk_info[key].set(template.format(**report))

The whole thing comes together in the main function (for lack of a better word):

def main(self):
    report = self.search()
    if report:
        self.update(self.parse(report))

Of course, this main function has to be linked to the Go button instead of search now.

Finally, I would guard the execution of the app with a if __name__ == "__main__" clause to allow importing of your module without executing it.


Final code:

import tkinter as tk
import pyowm
import time
import json
from datetime import datetime
from collections import OrderedDict


class WeatherInfo(tk.Tk):
    templates = OrderedDict([
        ('temp', 'Temperature: {temp:.1f} C'),
        ('humid', 'Humidity: {humid}%'),
        ('status', 'Status: {status}'),
        ('sunrise', 'Sunrise at {sunrise:%H:%M:%S}'),
        ('sunset', 'Sunset at {sunset:%H:%M:%S}'),
        ('day_length', 'Day length: {day_length:.1f} h'),
        ('night_length', 'Night length: {night_length:.1f} h')])

    def __init__(self):
        tk.Tk.__init__(self)
        self.wm_title('Weather')
        self.tk_info = {key: tk.StringVar(
            self, value='') for key in WeatherInfo.templates}

        self.ask = tk.LabelFrame(self, text='Location')
        self.ask.pack(fill='both', expand='yes')
        self.kw_label = tk.Label(self.ask, text='City:')
        self.kw_label.pack(side=tk.LEFT)
        self.kw = tk.Entry(self.ask)
        self.kw.pack(side=tk.LEFT)
        self.rb = tk.Button(self.ask, text='Go', command=self.main)
        self.rb.pack(side=tk.RIGHT)
        self.owm = pyowm.OWM('*CENSORED*')

        self.output = tk.LabelFrame(self, text='Information')
        self.output.pack(fill='both', expand='yes')
        self.labels = []
        for key in WeatherInfo.templates:
            self.labels.append(
                tk.Label(self.output, textvariable=self.tk_info[key]).pack())
        button = tk.Button(master=self, text='Quit', command=self._quit)
        button.pack(side=tk.BOTTOM)

    def search(self):
        obs = self.owm.weather_at_place(self.kw.get())
        try:
            return json.loads(obs.get_weather().to_JSON())
        except AttributeError:
            self.tk_info['temp'].set('Pick a city to display weather.')

    def parse(self, w):
        parsed_weather = {'temp': w['temperature']['temp'] - 273.15,
                          'humid': w['humidity'],
                          'status': w['status'],
                          'sunrise': datetime.fromtimestamp(w['sunrise_time']),
                          'sunset': datetime.fromtimestamp(w['sunset_time']),
                          'day_length': (w['sunset_time'] - w['sunrise_time']) / 3600,
                          'night_length': 24 - (w['sunset_time'] - w['sunrise_time']) / 3600}
        return parsed_weather

    def update(self, report):
        for key, template in WeatherInfo.templates.items():
            self.tk_info[key].set(template.format(**report))

    def main(self):
        report = self.search()
        if report:
            self.update(self.parse(report))

    def _quit(self):
        self.quit()
        self.destroy()

if __name__ == "__main__":
    app = WeatherInfo()
    app.mainloop()
\$\endgroup\$
0

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.