Simple interval timer in Python

Question

I created a script for interval timer with tkinter gui. Here the part that is displaying time looks to be done not in the best way. Any ideas would be greatly appreciated.

This timer is intended for timing HIIT/Tabata workouts. User provides working time, rest time, preparation time in seconds and integer number of intervals and upon hitting start button displays work/rest seconds countdown. After each interval a beep sound is played and a song after all of them are over. Also there options to change color of background and text.

from tkinter import *
from tkinter.colorchooser import *
from PIL import ImageTk, Image
import os

master = Tk()

def label_message(gt, wt, rt, it):

    res=[]

    for i in range(int(gt),-1,-1):
        res.append('Get Ready!: \n'+ str(i))
    res.append('')

    for i in range(int(it)):
        for j in range(int(wt),-1,-1):
            res.append('Round: \n' + str(i) +'\n Work : \n'+ str(j))
        res.append('')
        for k in range(int(rt),-1,-1):
            res.append('Round: \n' + str(i) +'\n Rest : \n'+ str(k))
        res.append('')

    res.append('Over!')

    return res

def window_color():
    color = askcolor()
    #print (color)
    widgets=[label1,label2,label3,label4,label5,master]
    for i in widgets:
        i.configure(background=color[1])

def text_color():
    color = askcolor()
    #print (color)
    widgets=[label1,label2,label3,label4,label5]
    for i in widgets:
        i.configure(fg=color[1])

def return_entry():

    wt = e1.get()
    rt = e2.get()
    gt = e3.get()
    it = e4.get()

    texts=label_message(gt, wt, rt, it)

    for i in texts:
        if i:
            master.after(1000, label5.config(text=i))
            master.update()
        else:
            master.bell()

    os.startfile('Jean Jacques Goldman - Asterix Et Obelix Contre Cesar (Generique Debut).mp3')


master.configure(background='white')
master.resizable()
master.title='Timer'

label1 = Label(master, text="Working Time", bg='white')
label2 = Label(master, text="Rest Time", bg='white')
label3 = Label(master, text="Get Ready Time", bg='white')
label4 = Label(master, text='Intervals', bg='white')
label5 = Label(master, text='time', height=5,  font=('Times New Roman', 15), bg='white')

img = ImageTk.PhotoImage(Image.open('clock.jpg'))
panel = Label(master, image = img)
panel.grid(column=0, row=0, rowspan=11, columnspan=3)

button1=Button(text='Window Color', command=window_color)
button1.grid(row=0, column=4, sticky=N+S+E+W)

button2=Button(text='Text Color', command=text_color)
button2.grid(row=0, column=5, sticky=N+S+E+W)

label1.grid(row=1, column=4)
label2.grid(row=2, column=4)
label3.grid(row=3, column=4)
label4.grid(row=4, column=4)
label5.grid(row=7, column=4, columnspan=2, rowspan=2)

e1 = Entry(master)
e2 = Entry(master)
e3 = Entry(master)
e4 = Entry(master)

e1.grid(row=1, column=5)
e2.grid(row=2, column=5)
e3.grid(row=3, column=5)
e4.grid(row=4, column=5)


button3=Button(text='Start', command=return_entry)
button3.grid(row=10, column=4, sticky=N+S+E+W)

button4=Button(text='Quit', command=master.quit)
button4.grid(row=10, column=5, sticky=N+S+E+W)

master.after(1000, None)
mainloop()

Answer 1

Don't use wildcard imports

I know that most tkinter tutorials show you to do from tkinter import *, but they are wrong. PEP8 recommends not use use wildcard imports, and I think that's especially true with tkinter.

Instead, use import tkinter as tk, and then prefix all tk classes with tk.. For example:

master = tk.Tk()
...
label1 = tk.Label(master, text="Working Time", bg='white')

You are using after incorrectly

This statement does not do what you think it does:

master.after(1000, label5.config(text=i))

The above is exactly the same as this:

result = label5.config(text=i)
master.after(1000, result)

My advice is to always have after call a function. So, write a function that takes two arguments, a label and a value, and updates the label with a value. Then, use after to call that function.

def update_label(label, value):
    label.configure(text=value)
...
master.after(1000, update_label, label5, i)

Give your widgets explicit masters / parents

Instead of this:

button1=Button(text='Window Color', command=window_color)

Do this:

button1=Button(master, text='Window Color', command=window_color)

While tkinter will use the default root window if you leave it out, PEP8 says that explicit is better than implicit. Plus, it makes it clear what your intentions are. Otherwise, someone reviewing your code might wonder if you're intentionally putting the widget in the root window or if it's a bug.

Remove useless code

I don't know what you think this is doing, but whatever that is, it's probably not doing it. You can remove this line of code:

master.after(1000, None)