3
\$\begingroup\$

I have developed a World Weather application using Tkinter in Python. The application retrieves current weather information and a 3-day weather forecast from the Weatherbit.io API based on user input. The application displays the weather data on a graphical user interface.

I would like feedback on the following aspects of my code:

  • Code structure and organization
  • Proper use of Tkinter widgets and layout management
  • Handling API requests and JSON data parsing Error
  • Handling and resilience
  • Optimization and efficiency considerations

Additionally, if you have any suggestions for code improvements, best practices, or alternative approaches, please let me know.

Link to full project on github

from datetime import datetime
import geocoder
from tkinter import *
import tkinter as tk
import requests
import calendar
from weather_icons import icons_day, icons_mini


def world_weather():

    # Create the window and set the basics
    root = Tk()
    root.title('World Weather by paichiwo')
    root.geometry('350x680+300+200')
    root.resizable(False, False)
    root.config(bg='black')

    # Colors
    l_blue = '#1581ef'
    d_blue = '#1167f2'

    # https://weatherbit.io/ API KEY
    api_key = ''

    def format_date_long(date):
        """ Format date to weekday, day month_name (Monday, 14 May) """
        date_object = datetime.strptime(date, '%Y-%m-%d')
        week_day = calendar.day_name[date_object.weekday()]
        month = date_object.month
        month_name = calendar.month_name[month]
        return f'{week_day[:3]}, {date_object.day} {month_name}'

    def format_date_short(date):
        """ Format date to day month_name (14 May) """
        date_object = datetime.strptime(date, '%Y-%m-%d')
        month = date_object.month
        month_name = calendar.month_name[month]
        return f'{date_object.day} {month_name[:3]}'

    def mtr_sec_to_km_per_hour(ms):
        """ Convert units - m/s to km/h """
        return ms * (1 / 1000) / (1 / 3600)

    def get_user_location():
        ip_location = geocoder.ip('me')
        city = ip_location.city
        country = ip_location.country
        return f"{city}, {country}"

    def get_current_weather(api_key, location):
        try:
            url = f'https://api.weatherbit.io/v2.0/current?city={location}&key={api_key}'
            current_data = requests.get(url).json()
        except requests.exceptions.JSONDecodeError:
            url = f'https://api.weatherbit.io/v2.0/current?postal_code={location}&key={api_key}'
            current_data = requests.get(url).json()
        return current_data

    def get_forecast_weather(api_key, location):
        try:
            f_url = f'https://api.weatherbit.io/v2.0/forecast/daily?city={location}&key={api_key}&days=4'
            forecast_data = requests.get(f_url).json()
        except requests.exceptions.JSONDecodeError:
            f_url = f'https://api.weatherbit.io/v2.0/forecast/daily?postal_code={location}&key={api_key}&days=4'
            forecast_data = requests.get(f_url).json()
        return forecast_data

    def update_current_weather_main_window(current_data):

        # Main window data
        city = current_data['data'][0]['city_name']
        country = current_data['data'][0]['country_code']
        code = current_data['data'][0]['weather']['code']
        current_temp = str(int(current_data['data'][0]['temp']))
        current_condition = current_data['data'][0]['weather']['description']
        local_time = format_date_long(current_data['data'][0]['datetime'][:-3])
        current_day_or_night = current_data['data'][0]['pod']

        # Update the labels
        city_info.config(text=f'{city}, {country}', fg='white', font=('Noto Sans', 12), justify='center', width=22)
        city_info.place(x=65, y=97)

        if current_day_or_night == 'd':
            weather_icon.config(file=icons_day[code])
            weather_label.place(x=97, y=130)
        else:
            weather_icon.config(file=icons_day[code])  # to update when night icons are made
            weather_label.place(x=97, y=130)

        temp.config(text=current_temp, justify='center', width=2)

        if len(current_temp) == 1:
            temp_symbol.config(text='°', justify='center')
            temp_symbol.place(x=205, y=290)
        else:
            temp_symbol.config(text='°', justify='center')
            temp_symbol.place(x=238, y=290)

        condition.config(text=current_condition, justify='center')
        date_info.config(text=local_time, justify='center')

    def update_current_weather_bottom_row(current_data):

        # Bottom row data
        current_feelslike = str(int(current_data['data'][0]['app_temp']))
        current_wind_speed = str(int(mtr_sec_to_km_per_hour(current_data['data'][0]['wind_spd'])))
        current_humidity = current_data['data'][0]['rh']
        current_cloud_coverage = str(int(current_data['data'][0]['clouds']))
        current_pressure = str(int(current_data['data'][0]['slp']))

        # Update the labels
        feelslike.config(text=f'{current_feelslike}°', justify='center')
        wind.config(text=f'{current_wind_speed} km/h', justify='center')
        humidity.config(text=f'{current_humidity}%', justify='center')
        cloud_coverage.config(text=f'{current_cloud_coverage}%', justify='center')  # change graphics !!!!!!
        pressure.config(text=f'{current_pressure} hPa', justify='center')
        textfield.delete(0, END)

    def create_forecast_data_list(forecast_data):

        forecast_data_list = []

        for forecast_days in forecast_data['data'][1:]:
            forecast_date = format_date_short(forecast_days['datetime'])
            forecast_avg_temp = str(int(forecast_days['temp']))
            forecast_avg_humidity = str(int(forecast_days['rh']))
            forecast_max_wind = str(int(mtr_sec_to_km_per_hour(forecast_days['wind_spd'])))
            forecast_code = forecast_days['weather']['code']

            forecast_data_list.append([forecast_date,
                                       forecast_avg_temp,
                                       forecast_avg_humidity,
                                       forecast_max_wind,
                                       forecast_code])
        return forecast_data_list

    def update_forecast_window(forecast_data_list):

        day_1_date.config(text=forecast_data_list[0][0], justify='center')
        day_1_temp.config(text=f'{forecast_data_list[0][1]}°', justify='center')
        day_1_humidity.config(text=f'{forecast_data_list[0][2]}%', justify='center')
        day_1_wind.config(text=f'{forecast_data_list[0][3]} km/h', justify='center')
        day_1_icon.config(file=icons_mini[forecast_data_list[0][4]])

        day_2_date.config(text=forecast_data_list[1][0], justify='center')
        day_2_temp.config(text=f'{forecast_data_list[1][1]}°', justify='center')
        day_2_humidity.config(text=f'{forecast_data_list[1][2]}%', justify='center')
        day_2_wind.config(text=f'{forecast_data_list[1][3]} km/h', justify='center')
        day_2_icon.config(file=icons_mini[forecast_data_list[1][4]])

        day_3_date.config(text=forecast_data_list[2][0], justify='center')
        day_3_temp.config(text=f'{forecast_data_list[2][1]}°', justify='center')
        day_3_humidity.config(text=f'{forecast_data_list[2][2]}%', justify='center')
        day_3_wind.config(text=f'{forecast_data_list[2][3]} km/h', justify='center')
        day_3_icon.config(file=icons_mini[forecast_data_list[2][4]])

    def get_weather():
        """ Connect to API, get data and update tkinter labels """

        if len(textfield.get()) > 0:
            location = textfield.get()
        else:
            location = get_user_location()

        try:
            current_data = get_current_weather(api_key, location)
            forecast_data = get_forecast_weather(api_key, location)
            forecast_list = create_forecast_data_list(forecast_data)
            update_current_weather_main_window(current_data)
            update_current_weather_bottom_row(current_data)
            update_forecast_window(forecast_list)
        except KeyError:
            city_info.config(text='Enter correct location', fg='yellow', justify='center', width=22)
        except requests.exceptions.ConnectionError:
            get_weather()

    # Create Search box
    search_image = PhotoImage(file='img/current_window.png')
    search_label = Label(image=search_image, bg='black')
    search_label.place(x=22, y=20)
    textfield = tk.Entry(root, cursor='hand2', justify='center', width=23, font=('Noto Sans', 11, 'bold'),
                         bg=d_blue, border=0, fg='white')
    textfield.place(x=60, y=67, height=25)
    textfield.focus()
    textfield.bind('<Return>', lambda event=None: search_button.invoke())
    search_icon = PhotoImage(file='img/magnifying_glass.png')
    search_button = Button(image=search_icon, activebackground=l_blue, borderwidth=0,
                           bg=d_blue, command=get_weather)
    search_button.place(x=268, y=65)

    # Create Current Weather labels

    city_info = Label(text='Enter city name or postcode', font=('Noto Sans', 10), bg=l_blue, fg='white')
    city_info.place(x=90, y=97)
    weather_icon = PhotoImage(file='img/splash_icon.png')
    weather_label = Label(root, image=weather_icon, bg=l_blue)
    weather_label.place(x=105, y=157)
    temp = Label(text='', font=('Noto Sans', 85, 'bold'), bg=l_blue, fg='white')
    temp.place(x=104, y=280, height=111)
    temp_symbol = Label(text='', font=('Noto Sans', 20, 'bold'), height=1, bg=l_blue, fg='white')
    temp_symbol.place(x=206, y=290, width=15)
    condition = Label(text='', font=('Noto Sans', 11), bg=l_blue, fg='white', width=27)
    condition.place(x=53, y=390, height=30)
    date_info = Label(text='', font=('Noto Sans', 8), bg=l_blue, fg='white', width=30)
    date_info.place(x=69, y=418, height=15)

    feelslike = Label(text='', font=('Noto Sans', 8, 'bold'), bg=l_blue, fg='white', width=3)
    feelslike.place(x=63, y=487, height=15)
    wind = Label(text='', font=('Noto Sans', 8, 'bold'), bg=l_blue, fg='white', width=7)
    wind.place(x=100, y=487, height=15)
    humidity = Label(text='', font=('Noto Sans', 8, 'bold'), bg=l_blue, fg='white', width=4)
    humidity.place(x=163, y=487, height=15)
    cloud_coverage = Label(text='', font=('Noto Sans', 8, 'bold'), bg=l_blue, fg='white', width=5)
    cloud_coverage.place(x=206, y=487, height=15)
    pressure = Label(text='', font=('Noto Sans', 8, 'bold'), bg=l_blue, fg='white', width=7)
    pressure.place(x=249, y=487, height=15)

    # Create Forecast Weather labels
    day_1_date = Label(text='', font=('Noto Sans', 9, 'bold'), bg='black', fg='white', width=10)
    day_1_date.place(x=32, y=535, height=20)
    day_1_temp = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_1_temp.place(x=37, y=555, height=20)
    day_1_humidity = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_1_humidity.place(x=37, y=575, height=20)
    day_1_wind = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_1_wind.place(x=37, y=595, height=20)
    day_1_icon = PhotoImage(file='img/dummy_mini.png')
    day_1_icon_label = Label(root, image=day_1_icon, bg='black')
    day_1_icon_label.place(x=53, y=618)

    day_2_date = Label(text='', font=('Noto Sans', 9, 'bold'), bg='black', fg='white', width=10)
    day_2_date.place(x=135, y=535, height=20)
    day_2_temp = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_2_temp.place(x=140, y=555, height=20)
    day_2_humidity = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_2_humidity.place(x=140, y=575, height=20)
    day_2_wind = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_2_wind.place(x=140, y=595, height=20)
    day_2_icon = PhotoImage(file='img/dummy_mini.png')
    day_2_icon_label = Label(root, image=day_2_icon, bg='black')
    day_2_icon_label.place(x=156, y=618)

    day_3_date = Label(text='', font=('Noto Sans', 9, 'bold'), bg='black', fg='white', width=10)
    day_3_date.place(x=234, y=535, height=20)
    day_3_temp = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_3_temp.place(x=239, y=555, height=20)
    day_3_humidity = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_3_humidity.place(x=239, y=575, height=20)
    day_3_wind = Label(text='', font=('Noto Sans', 8, 'bold'), bg='black', fg='white', width=10)
    day_3_wind.place(x=239, y=595, height=20)
    day_3_icon = PhotoImage(file='img/dummy_mini.png')
    day_3_icon_label = Label(root, image=day_3_icon, bg='black')
    day_3_icon_label.place(x=255, y=618)

    root.mainloop()


if __name__ == '__main__':
    world_weather()


\$\endgroup\$
2
  • \$\begingroup\$ This code is unmaintainable as currently stated. There's no answers yet, so you still have time to revise it. Write a test for mtr_sec_to_km_per_hour, for example. The testcase will need to import world_weather. But it can't, due to the side effects on root and textfield and so on. Bury those within a def function, and have an if __name__ == "__main__": guard protect the execution of that. Also, the get_weather function is way way too long -- use Extract Helper to break it up into several smaller functions. \$\endgroup\$
    – J_H
    May 23, 2023 at 3:46
  • 1
    \$\begingroup\$ code revised and if __name__ == '__main__': guard added \$\endgroup\$
    – Paichiwo
    May 23, 2023 at 11:42

1 Answer 1

2
\$\begingroup\$

Thanks, the __main__ guard looks good.

Pep-8 asks that you group similar imports together. One could follow the English instructions there, but it's far simpler to just run "$ isort ." and be done with it. How does that improve readability? Well, I was initially viewing the unfamiliar calendar package as a pypi dependency I would need to look up, but now I see it's a batteries included thing which can do some of what .strftime() does. Also requests now appears down where we'd expect it, with the other pypi deps.

from tkinter import *

Please avoid * star imports. Yes, I know, the tutorials all do that, because tk offers a ton of cool widgets and they want you to be able to try them out, friction free.

But at this point you're getting serious about your craft, you're ready to review and merge to main, so it's time spruce this up a bit. And it's not that hard. Just turn it into an import of Tk and let lint or your IDE tell you about missing references:

$  ruff --format=json geo/zone/so/world_weather_app.py | jq '.[].message' | sort | uniq -c
   1 "Undefined name `Button`"
  27 "Undefined name `Label`"
   6 "Undefined name `PhotoImage`"

giving us:

from tkinter import Button, Label, PhotoImage, Tk

def world_weather():
    # Create the window and set the basics

This is a perfectly nice # comment at top of function. But in python we spell it this way instead:

def world_weather():
    """Create the window and set the basics."""

Now help(main) will show something helpful. But more importantly you want to get into the habit of using """docstrings""" to help you think carefully about APIs you design, and to explain parameter details to the Gentle Reader.


    root.geometry('350x680+300+200')

The root window is well-named.

In the argument we see four magic numbers. Now, maybe this is perfect as written, maybe we had to express these numbers just once in the program and this is the perfect manner to do so. The (300, 200) offset probably fits that. The width and height dimensions are likely to interact with other things, so we probably want to instead express it as f"{width}x{height}+300+200". Let's keep reading, and we'll keep an eye on that.


    # https://weatherbit.io/ API KEY
    api_key = ''

No. A secret like an API key never belongs in source code. Put it in an os.getenv() environment variable, or in a .toml config file.


    def format_date_long(date):
        """ Format date to weekday, day month_name (Monday, 14 May) """

Woo hoo! These functions come with docstrings, excellent.

You are nesting functions within your main world_weather() function. I don't recommend that. Just pop them out by one indent, so they are up at top-level. Why? Nested functions can be useful. But they share the enclosing namespace, so there's more coupling than necessary which can start to look a lot like global variables. In essence, all the variables one level up are being implicitly passed in as arguments. Avoid that, unless you have a good reason for doing it and you write down that reason.


    def mtr_sec_to_km_per_hour(ms):

The docstring and the pair of conversion factors are very clear, thank you. Recommend you rename the function and also the argument:

    def mtr_per_sec_to_km_per_hour(m_per_s):

The / "per" matters, and ms risks being confused with an SI unit.


    def get_current_weather(api_key, location):
        try:
            url = f'https://api.weatherbit.io/v2.0/current?city={location}&key={api_key}'
            current_data = requests.get(url).json()

This works. But prefer to habitually use this approach:

            url = f'https://api.weatherbit.io/v2.0/current'
            params = dict(city=location, key=api_key)
            current_data = requests.get(url, params=params).json()

Why? At some point quoting / escaping odd characters, like punctuation, is going to matter, and you don't want to do that yourself. Better to let .get() worry about those details.

Also, kudos on catching something narrow like JSONDecodeError, that's very nice, it clearly reveals Author's Intent without accidentally masking some other problem. Sometimes we need to await remote parsing from the distant endpoint. But here it seems like it's just too simple to test

if re.search(r"^\d{5}$", location):

so we ask the right question, zipcode vs city name, from the get go.

You are perhaps a bit quick on the draw with that .json() call. Consider GETting a resp response object and carefully examining whether it has 200 success status before attempting the JSON decode.


Hmmm, same thing going on with get_forecast_weather().

Consider defining a class Location: which knows the difference between zipcode and place name, and which knows how to turn a base_url into a properly phrased query.


        city = current_data['data'][0]['city_name']

I see a lot of tedious ['data'][0] expressions being repeated below this. Recommend you change the initial assignment so current_data is already pointing down at the item of interest. Then you could access the name with current_data['city_name']

        local_time = format_date_long(current_data['data'][0]['datetime'][:-3])

Please rephrase this, with the aid of a trivial helper:

        local_time = format_date_long(discard_tz(current_data['datetime']))

At least, I think the intent was to strip a trailing timezone.


        city_info.place(x=65, y=97)
            ...
            weather_label.place(x=97, y=130)
            ...
            temp_symbol.place(x=205, y=290)

Ok, lots of magic numbers related to layout. Maybe this is perfect as-is. Or maybe you'd like to define LEFT_MARGIN = 65 and then talk about LEFT_MARGIN + 32, that sort of thing. So if you later squeeze something into the left margin it's straightforward for everything else to shuffle over to the right a bit. Or perhaps express these as some decimal fraction of WIDTH.

Better still, maybe you'd like to define tiny widgets and let tk worry about packing them in a visually attractive way.


The formatting expression str(int( ... )) appears often enough that you might break out a helper for it.

Or consider phrasing it as f"{int( ... }"


update_forecast_window() raises some DRY issues. Isn't there any way we could solve the day_1 problem just once? and recycle that solution for days 2 and 3?

See "# Create Forecast Weather labels" below for similar tedium.


        try:
            current_data = get_current_weather(api_key, location)
            forecast_data = ...
            forecast_list = ...
        except KeyError:
            city_info.config(text='Enter correct location', ...)
        except requests.exceptions.ConnectionError:
            get_weather()

Recursing back to yourself on a 500 retry isn't the right approach. You want a while loop here. It will avoid stack overflow. Notify the user of what's happening, perhaps by displaying a retry_count. Consider wrapping an API access function with tenacity to handle retries.

Catching KeyError works, but it doesn't quite feel right. Clearly there could be a typo in the city name, and then the Right Thing still needs to happen. I suspect your currently designed Public API isn't making exactly the right promises for this to work smoothly. Let's step back a moment and consider a text-based application. We query weatherbit.io for a forecast, receive 200 or 404, and display "sunny!" or "no such city as Alberkerkie" -- the str can represent both a forecast and an error condition. Now we get fancier than text, and pass back either a Forecast object or a tk display widget. Whichever you choose (I recommend the former), it again should be able to represent and display both the good and bad conditions.


    # Create Search box
    search_image = PhotoImage(file='img/current_window.png')

The # comment suggests that you might want to break out a def create_search_box(): helper.

The file= reference works, but only if caller has carefully cd'd to the right place, so it's a bit fragile. Consider adopting this approach:

    def _base_dir():
        return Path(__file__ + "/..").resolve()

    ...
    search_image = PhotoImage(file=_base_dir() / 'img/current_window.png')

It's not exactly right, as it doesn't cope with an app packaged as a .ZIP, but it's more robust than the current code.


    root.mainloop()

The get_weather() function is on the big side, with good reason. Consider making it the caller's responsibility to invoke .mainloop(), to improve the testability of get_weather().


This app achieves its design goals. On the whole it looks pretty good, with clear straightforward code.

I would be willing to delegate or accept maintenance tasks on this codebase.

\$\endgroup\$
4
  • \$\begingroup\$ Firstly, it was a pleasure to read your answer, however, Im still quite a beginner, but ill apply what I can. What do you mean by accepting tasks on this project, a mentoring on github? That would be great. Maybe I could move the whole thing into Class, which really fits this project, i got to some start but got lost on the way. \$\endgroup\$
    – Paichiwo
    May 23, 2023 at 21:35
  • \$\begingroup\$ Sure, that would be fine, hit me up! When I first approached this site I was like, 'what? zero to five stars?" Wazzup with that? But then I realized there are two objective measures we're trying to evaluate before saying it's OK to merge down to main: (1) looking at this code, is it obviously correct? and (2) is it maintainable by the random undergrad intern we hire next summer? So there are two objective measures. I was reporting (True, True) on each of them. \$\endgroup\$
    – J_H
    May 24, 2023 at 1:16
  • \$\begingroup\$ 1. You are welcome to make any changes and create a pull request. In fact, I would greatly appreciate it. 2. How can I provide you with the updated version of the code? Should I create a new question or respond to this one? Alternatively, share a link to the new file on GitHub? \$\endgroup\$
    – Paichiwo
    May 24, 2023 at 11:32
  • \$\begingroup\$ I left a PR for you. There are some review items you should probably attend to first. \$\endgroup\$
    – J_H
    May 24, 2023 at 17:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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