Time Converter - Python Tkinter - General comments on code

Question

I have been learning python since being in lockdown, this is the first Tkinter application I've built without consulting the internet or seeking help anywhere.

It is a time converter where you can select the input unit via a checkbox and the other entry windows will fill with the correct conversion.

I'd like to hear feedback on how it's written, in terms of if I have demonstrated best practice or areas where I could have used less code.

from tkinter import * 
from tkinter.ttk import*
root = Tk() 
Second = IntVar() 
Minute = IntVar() 
Hour = IntVar()
Var1 = IntVar() 
Var2 = IntVar() 
Var3 = IntVar()
def ConvertTime():
 if Var1.get() == 1:
 Minute.set(Second.get()/60)
 Hour.set(Second.get()/3600)
 
 if Var2.get() == 1:
 Second.set(Minute.get()*60)
 Hour.set(Minute.get()*0.01667)
 if Var3.get() == 1:
 Second.set(Hour.get()*3600)
 Minute.set(Hour.get()*60)
class GUI:
 def __init__(self, master):
 self.master = master
 master.title('Time Converter')
 master.geometry('+600+300')
 master.bttn = Button(root, text = 'Convert', command = ConvertTime)
 master.bttn.grid(pady= 5, padx=20, column=1, row=3)
 master.ChooseSecond = Checkbutton(root, text = 'Seconds', variable=Var1, onvalue = 1, offvalue = 0)
 master.ChooseSecond.grid(pady=5, padx=20, column=0, row=0)
 master.entry1 = Entry(root, textvariable=Second)
 master.entry1.grid(pady=5, padx=20, column=0, row=1)
 master.ChooseMinute = Checkbutton(root, text = 'Minutes', variable=Var2, onvalue = 1, offvalue=0)
 master.ChooseMinute.grid(pady=5, padx=20, column=1, row=0)
 master.entry2 = Entry(root, textvariable=Minute)
 master.entry2.grid(pady=5, padx=20, column=1, row=1)
 master.ChooseHour = Checkbutton(root, text = 'Hours',variable=Var3, onvalue=1, offvalue=0)
 master.ChooseHour.grid(pady=5, padx=20, column=2, row=0)
 master.entry3 = Entry(root, textvariable=Hour)
 master.entry3.grid(pady=5, padx=20, column=2, row = 1)
def main():
 GUI(root)
 root.mainloop()
main()

Answer 1

• Avoid putting variables in the global namespace
• Use a more meaningful variable name than Var1
• With your current usage, Varx variables should be boolean variables, not integer variables, due to them connecting to checkboxes
• Use radio buttons rather than checkboxes, because you really want exclusive selection
• No reason not to accept floats instead of ints for your entry boxes
• Generalize a series of conversion factors so that your division and multiplication only need to be expressed once
• No need to set your controls as members on master
• Avoid calling things master; it's 2021
• Centralize the expression of your padding quantities, since they're all the same
• Separate your business logic from your presentation

Suggested

from enum import Enum
from functools import partial
from tkinter import Button, DoubleVar, Entry, Grid, IntVar, Radiobutton, Tk
from typing import Callable, Dict, Iterable
FACTORS = (1, 60, 60 * 60)
class TimeComponent(Enum):
 SECOND = 0
 MINUTE = 1
 HOUR = 2
 def get_factor(self) -> int:
 return FACTORS[self.value]
class GUIVariable:
 def __init__(
 self,
 root: Tk,
 component: TimeComponent,
 selected_component: IntVar,
 ):
 self.check_button = Radiobutton(
 root,
 text=component.name.title(),
 variable=selected_component,
 value=component.value,
 )
 value = DoubleVar()
 self.get_value: Callable[[], float] = value.get
 self.set_value: Callable[[float], None] = value.set
 self.entry = Entry(root, textvariable=value)
class TimeController:
 def __init__(self, component: TimeComponent, gui: GUIVariable):
 self.get_factor: Callable[[], int] = component.get_factor
 self.gui = gui
 @property
 def normalized(self) -> float:
 return self.gui.get_value() * self.get_factor()
 @normalized.setter
 def normalized(self, x: float) -> None:
 self.gui.set_value(x / self.get_factor())
 def adjust_others(self, others: Iterable['TimeController']):
 value = self.normalized
 for other in others:
 if other is not self:
 other.normalized = value
class GUI:
 def __init__(self):
 root = Tk()
 self.run: Callable[[], None] = root.mainloop
 root.title('Time Converter')
 root.geometry('+600+300')
 padded_grid = partial(Grid.grid, pady=5, padx=20)
 convert_button = Button(root, text='Convert', command=self.convert_time)
 padded_grid(convert_button, column=1, row=3)
 self.selected_var = IntVar()
 self.controllers: Dict[TimeComponent, TimeController] = {}
 for component in TimeComponent:
 var = GUIVariable(root, component, self.selected_var)
 padded_grid(var.check_button, column=component.value, row=0)
 padded_grid(var.entry, column=component.value, row=1)
 controller = TimeController(component, var)
 self.controllers[component] = controller
 @property
 def current_controller(self) -> TimeController:
 return self.controllers[
 TimeComponent(self.selected_var.get())
 ]
 def convert_time(self) -> None:
 self.current_controller.adjust_others(self.controllers.values())
if __name__ == '__main__':
 GUI().run()

Alternative

An alternate GUI that is fairly simplified: don't have any selectors or buttons, and when one field changes, update the others.

from enum import Enum
from functools import partial
from tkinter import DoubleVar, Entry, Grid, Label, Tk, TclError
from typing import Callable, Dict, Iterable
FACTORS = (1, 60, 60 * 60)
class TimeComponent(Enum):
 SECOND = 0
 MINUTE = 1
 HOUR = 2
 def get_factor(self) -> int:
 return FACTORS[self.value]
class GUIVariable:
 def __init__(
 self,
 root: Tk,
 component: TimeComponent,
 trace: Callable[['GUIVariable'], None],
 ):
 value = DoubleVar()
 value.trace('w', self.trace_inner)
 self.trace = trace
 self.component = component
 self.get_value: Callable[[], float] = value.get
 self.set_value: Callable[[float], None] = value.set
 self.label = Label(root, text=component.name.title())
 self.entry = Entry(root, textvariable=value)
 def trace_inner(self, name: str, _: str, mode: str) -> None:
 self.trace(self)
class TimeController:
 def __init__(self, component: TimeComponent, gui: GUIVariable):
 self.get_factor: Callable[[], int] = component.get_factor
 self.gui = gui
 @property
 def normalized(self) -> float:
 return self.gui.get_value() * self.get_factor()
 @normalized.setter
 def normalized(self, x: float) -> None:
 self.gui.set_value(x / self.get_factor())
 def adjust_others(self, others: Iterable['TimeController']):
 try:
 value = self.normalized
 except TclError:
 return
 for other in others:
 if other is not self:
 other.normalized = value
class GUI:
 def __init__(self):
 root = Tk()
 self.run: Callable[[], None] = root.mainloop
 root.title('Time Converter')
 root.geometry('+600+300')
 padded_grid = partial(Grid.grid, pady=5, padx=20)
 self.controllers: Dict[TimeComponent, TimeController] = {}
 for component in TimeComponent:
 var = GUIVariable(root, component, self.on_change)
 padded_grid(var.label, column=component.value, row=0)
 padded_grid(var.entry, column=component.value, row=1)
 controller = TimeController(component, var)
 self.controllers[component] = controller
 def on_change(self, var: GUIVariable) -> None:
 controller = self.controllers[var.component]
 controller.adjust_others(self.controllers.values())
if __name__ == '__main__':
 GUI().run()

simplified

Answer 2

One issue I can see on first glance is the *. It's considered bad practice to import everything like that, and here is why from this Stack Overflow post:

• Because it puts a lot of stuff into your namespace (might shadow some other object from previous import and you won't know about it).
• Because you don't know exactly what is imported and can't easily find from which module a certain thing was imported (readability).
• Because you can't use cool tools like pyflakes to statically detect errors in your code.

• Reinderien: Overall your feedback is correct - splat-import should generally be discouraged. However, how does this interfere with static analysis? I think it probably doesn't?
• Chocolate: @Reinderien I didn't say anything about the static analsis. The OP asked "I'd like to hear feedback on how it's written, in terms of if I have demonstrated best practice".
• Reinderien: Then what did you mean by statically detect errors in your code ?