Weather App- Python

Question

This is my very basic Weather App. Would appreciate your opinion mainly about style, writing code etc. I know there is not much functionality to this app, but I have created it just to practice and to share with you guys so you could possibly point weak points I need to focus on as a programmer. As I am learning mainly from materials I find online, the opinion of professionals will be much appreciated so I can go in the right direction.

import tkinter as tk
import requests, json, os
from tkinter import StringVar, ttk
from pathlib import Path


class Window(tk.Tk):
    def __init__(self, database: object, icons: str) -> None:
        super().__init__()
        self._db = database
        self._icons = icons
 
        self.enter_city()
        self.update_vars()
        self.labels()
        self.weather_img()
        # auto update function, 'while typing' style    
        self.update_data()
        
        self.geometry('350x450')
        self.title('Weather App')
        self.iconbitmap(f'{icons}\\icon.ico')


    def weather_img(self) -> None:
        # set weather image
        self.icon = StringVar()
        self.icon.set(WeatherData.get_icon_code(self.weather))
        # weather icon path
        weather_icon = Path(f'{icons}\\{self.icon.get()}.png')
        # ensure during changing city when icon code not availabe there is no error, refresh icon on the screen
        if weather_icon.is_file():
            self.weather_image = tk.PhotoImage(file=f'{icons}\\{self.icon.get()}.png')
        else:
            self.weather_image = tk.PhotoImage(file=f'{icons}\\refresh.png')
        self.image_lbl = ttk.Label(self, image=self.weather_image)
        self.image_lbl.place(x=45, y=35)            


    def enter_city(self) -> None:
        # City entry
        self.city_ent = ttk.Entry(self, justify='center', width=50)
        self.city_ent.place(x=20, y=10)
        self.city_var = StringVar(value='Warsaw')


    def update_vars(self) -> None:
        # Get weather data for City
        self.weather = WeatherData.get_weather(self._db, self.city_var.get())
        
        # set vars for auto update
        self.temp = StringVar()
        self.temp.set(WeatherData.get_temp(self.weather))
        self.wind = StringVar()
        self.wind.set(WeatherData.get_wind_direction(self.weather))
        self.prediction = StringVar()
        self.prediction.set(WeatherData.get_predicted_weather(self.weather))


    def labels(self) -> None:
        self.city_lbl = ttk.Label(self, textvariable=self.city_var, width=35, font=('Helvetica', 12, 'bold'), anchor='center')
        self.city_lbl.place(x=10, y=290)
        self.city_ent.configure(textvariable=self.city_var)

        self.temp_lbl = ttk.Label(self, text='Temperature:', font=('Helvetica', 11))
        self.temp_lbl.place(x=10, y=330)

        self.wind_lbl = ttk.Label(self, text='Wind direction:', font=('Helvetica', 11))
        self.wind_lbl.place(x=10, y=360)

        self.prediction_lbl = ttk.Label(self, text='Predicted weather:', font=('Helvetica', 11))
        self.prediction_lbl.place(x=10, y=390)

        self.temp_val_lbl = ttk.Label(self, textvariable=self.temp, font=('Helvetica', 11))
        self.temp_val_lbl.place(x=220, y=330)
        
        self.wind_val_lbl = ttk.Label(self, textvariable=self.wind, font=('Helvetica', 11))
        self.wind_val_lbl.place(x=220, y=360)

        self.prediction_val_lbl = ttk.Label(self, textvariable=self.prediction, font=('Helvetica', 11))
        self.prediction_val_lbl.place(x=220, y=390)
        
        
    def update_data(self) -> None:
        self.weather = WeatherData.get_weather(self._db, self.city_var.get())
        self.temp.set(WeatherData.get_temp(self.weather))
        self.wind.set(WeatherData.get_wind_direction(self.weather))
        self.prediction.set(WeatherData.get_predicted_weather(self.weather))
        # destroy icon label, so new one won't be on top of old one
        self.image_lbl.destroy()
        # refresh icon label
        self.weather_img()
        self.after(1000, self.update_data)



class WeatherData:
    def __init__(self, key: str) -> None:
        self._key = key

    # excepting KeyError for autoupdate. App won't stop working while typing new city name

    def get_weather(self, city: str) -> list:
        url = f'https://api.openweathermap.org/data/2.5/weather?q={city}&units=metric&appid={self._key}'
        response = requests.get(url)
        data = response.json()
        return data


    def get_temp(data: list) -> str:
        try:
            celsius = data['main']['temp']
            return str(round(celsius))
        except KeyError:
            pass


    def get_predicted_weather(data: list) -> str:
        try:    
            sky = data['weather']
            for item in sky:
                for key, value in item.items():
                    if key == 'main':
                        return str(value)
        except KeyError:
            pass


    def get_wind_direction(data: list) -> str:
        try:    
            wind = data['wind']['deg']
            if wind == 0:
                return 'North'
            elif wind > 0 and wind < 90:
                return 'North East'
            elif wind == 90:
                return 'East'
            elif wind > 90 and wind < 180:
                return 'South East'
            elif wind == 180:
                return 'South'
            elif wind > 180 and wind < 270:
                return 'South West'
            elif wind == 270:
                return 'West'
            else:
                return 'North West'
        except KeyError:
            pass


    # get icon code to display right weather icon
    def get_icon_code(data: list) -> str:
        try:    
            info = data['weather']
            for item in info:
                for key, value in item.items():
                    if key == 'icon':
                        return value
        except KeyError:
            pass


if __name__ == "__main__":
    _current_path = os.path.dirname(__file__)
    
    # Free API key
    with open(f'{_current_path}\\settings\\apiconfig.json', 'r') as f:
        api_file = json.load(f)
        for key, value in api_file.items():
            if key == 'key':
                _api_key = value
    
    # icons folder
    icons = f'{_current_path}\\icons'
    
    db = WeatherData(_api_key)
    Window(db, icons).mainloop()

Answer 1

• Don't hint database as an object; hint it as a WeatherData. To resolve the missing symbol, move Window below WeatherData. If for whatever reason you weren't able to do that, you could also hint it in string syntax, i.e. 'WeatherData'.
• Avoid inheriting from tk.Tk and instead put it in a member variable ("has-a", not "is-a"). I say this because you don't override any of the parent functions and you don't reference any of its member variables from what I see.
• Rather than hard-coding backslashes in your paths, make icons a pathlib.Path, and use the / operator to get to icon.ico
• self.icon isn't referenced outside of weather_img so it doesn't need to be a member variable; similar for the member variables weather_image, city_lbl, etc.
• Repeated expressions like this:

    ttk.Label(self, text='Temperature:', font=('Helvetica', 11))

can be reduced by

label = partial(ttk.Label, self, font=('Helvetica', 11))

self.temp_lbl = label(text='Temperature:')
# etc.

• It's a good idea to represent WeatherData as a class, but not such a good idea to have most of its member functions accept a data parameter. If you move get_weather outside of the class and pass its result to WeatherData's constructor. Save it to a member variable. Modify get_temp to have a self parameter, no data parameter, and use its member variable; similar with other functions. This way, WeatherData is actually meaningful as a class, and is effectively immutable.
• Do not format your query parameters into your URL. Pass them in the params dict argument to get().
• Use the return value of get, the response, in a with.
• Call response.raise_for_status().
• Since you have exception handling around data['main']['temp'], the return type can't be str, but instead Optional[str]. Similar for other functions.
• It's good that you have a main guard, but it's not enough. All of those variables are still in global scope. Make a main function and call it from your main guard.

Answer 2

• Destroying and re-creating the entire icon label feels like an awkward solution. Isn't it possible to keep the label as it is and only replace its image? I believe something like self.image_lbl.configure(image=self.weather_image) might do that

  • I'm also wondering if we actually need to do that manually. We might be able to use self.icon.trace_add to automatically update the icon when that variable changes. This might look a bit like so:

def enter_city(self) -> None:
    self.icon = StringVar()

    def icon_change_callback(_: str, _name: str, _mode: str):
        icon_path = Path(self._icons, f"{self.icon.get()}.png")

        if icon_path.is_file():
            self.weather_image = tk.PhotoImage(file=icon_path)
        else:
            self.weather_image = tk.PhotoImage(file=Path(self._icons, "refresh.png"))

        self.image_lbl.configure(image=self.weather_image)

    self.icon.trace_add("write", icon_change_callback)

    self.image_lbl = ttk.Label(self)
    self.image_lbl.place(x=45, y=35)

• That still ends up re-loading the entire icon file every second, though. That's not ideal. It'd probably be better to keep the PhotoImages around somewhere so we can look them up without having to bother wish disk I/O

• We might be able to use callbacks with self.city_var as well - that might let us update right away when the variable gets updated, while not having to update every second the rest of the time. The weather doesn't usually change that fast after all

• Why is WeatherData a class anyway? Most of the methods don't interact with the class at all, so they could just as easily be standalone functions. The only exception is get_weather, but if we only have one method, we usually aren't any better off by using a class than we'd be if we'd just used a function

• Those loops in get_icon_code and get_predicted_weather feel awkward. It seems to me like they can be simplified to data['weather'][0]['main'] - am I missing something?

  • If we can do that, most of those helper functions turn into one-liners that just get data from a known location in a dict. As much as I like functions, I'm not sure we actually need to keep those

• get_wind_direction feels like it has two jobs - getting a wind angle from an API result and turning an angle into a compass direction. Those aren't that closely related, and kind of feel like they could belong in two different functions

• Similarly, update_vars is responsible for creating variables, while update_data is clearly responsible for updating the program's state. So I'd say we could get rid of the updating part of update_vars, maybe rename it to create_vars or something

• Speaking of names, all of Window's methods exist to perform actions, and should probably be named after the actions they perform. labels and weather_img feel less descriptive than create_labels and create_weather_image. And while enter_city and update_vars do describe actions, those are not the actions those functions perform - enter_city creates an entry field, and as mentioned update_vars doesn't update existing variables but instead creates new ones

• The data parameters of WeatherData's methods all seem to have the wrong type - the type hints say list, but they're dicts

  • Those functions also claim to always return str, but they may return None. Either allow them to raise those exceptions instead of swallowing them, change the return type to typing.Optional[str], or make them return some kind of str even if the lookup fails

• weather_img depends on the global variable icons, which I don't think we want. We already save that variable as self._icons during __init__, so I see no reason not to use that instead

• Side note - it feels a bit weird to me that get_wind_direction only returns "north", "south", "east" and "west" if the angle is exactly 0, 180, 90 or 270 degrees - I'd say 89 degrees is more east than north-east

• While building a query string by hand works fine, it's often neater to use the params argument of request.get's params options:

requests.get('https://api.openweathermap.org/data/2.5/weather', params={
    "q": city,
    "units": "metric",
    "appid": key
})

• Chained comparisons like 90 < angle and angle < 180 can be simplified to 90 < angle < 180

• Different operating systems have different ways of separating the parts of a file path. For portability, it might be better to not provide the path as a single string, but instead use Path to combine the pieces like Path(_current_path, 'icons') instead of f"{_current_path}\\icons"