Python tkinter GUI

Question

I'm making a little Python script that reads a text file and puts it on a HTML page. I haven't finished that part yet, but I've been working on the GUI with tkinter. Is my code sloppy?

from Tkinter import *
import time
from datetime import datetime
#init variables
global r
r = 0
global delay
delay = 1000
#for testing
global lat
lat = "41.5345"
global lon
lon = "-85.343"
global zoom
zoom = 0
global mapDir
mapDir = ""
global mapDirEnabl
mapDirEnabl = 0
#clear log file
log = open("xpyMapLog.txt", "w")
log.write("")
log.close()
global log
log = open("xpyMapLog.txt", "a")
logStartMessage = "RUN TIME EXECUTED AT: %s \n" % str(datetime.now())
log.write(logStartMessage) 

#error check
class safe:
    def __init__(self, function):
        self.function = function

    def __call__(self, *args):
        try:
            return self.function(*args)
        except Exception, e:
            errorMsg = "Error: %r" % str(e)
            log.write(errorMsg)  
            print "Error: %s" % e
        #
    #
#

#open data file
@safe
def openData():
    global Data
    Data = open("Data.txt", "r")
#
openData()


#init root window
root = Tk()
root.geometry("550x300+200+200")
root.title("XPYmap for x-plane 10")


#init functions         

@safe
def mainLoop():
    if r == 1:
        #**************************************
        #DATA READING AND WRITING TO MAP FILE!*
        #**************************************
        print "Hello"       
        #im not done with the data reading yet so just pretend its here

        #######################################
    #       
#


@safe
def start():
    global r
    r = 1
    mainLoop()
#
@safe
def stop():
    global r
    r = 0
    posLabl["text"] = "Position: NOT RUNNING"
#
@safe
def setZoom(z):
    global zoomlabel
    zoom = int(z)
    curntZoomLabl["text"] = "Current zoom level: %s" % zoom
#
@safe
def setDelay(d):
    print "delay"
    global delay    
    delay = int(d)
    delayInSec = delay / 1000 # get seconds from milisec
    curntDelayLabl["text"] = "Current update delay: %s seconds." % delayInSec
#

@safe
def chooseMapDir(): 
    global mapDirEnabl
    global mapDir
    if mapDirEnabl == 0:
        mapDirEnabl = 1
        mapDirStat["text"] = "Custom map directory is currently: ENABLED!"
        mapDir = str(mapDirEntry.get())
        print mapDir
    elif mapDirEnabl == 1:
        mapDirEnabl = 0
        mapDirStat["text"] ="Custom map directory is currently: DISABLED!" 
        mapDir = ""
        print mapDir
    #

#


#widgets

posLabl = Label(root, text = "Position: NOT RUNNING")
butStart = Button(root, text = "Start", command = start)
butStop = Button(root, text = "Stop", command = stop)
zoomLabl = Label(root, text = "Click the zoom level")
curntZoomLabl = Label(root, text = "Current zoom level:")
delayLabl = Label(root, text = "Select update delay")
curntDelayLabl = Label(root, text = "Current update delay: NONE")
mapDirLabl = Label(root, text = "ADVANCED ONLY! If you want to save the map file to another directory, enter it in below.")
mapDirBut = Button(root, text = "enable/disable directory", command = chooseMapDir)
mapDirStat = Label(root, text = "Custom map directory is currently: DISABLED!")
mapDirEntry = Entry(root, width = 60)

#column span for buttons
colspan = 1

#zoom with grid

bz1 = Button(root, text = "1", width=2, command = lambda:setZoom("1"))  
bz1.grid(row = 5, column = 0, columnspan = colspan)       
bz2 = Button(root, text = "2", width=2, command = lambda:setZoom("2"))   
bz2.grid(row = 5, column = 1, columnspan = colspan  ) 
bz3 = Button(root, text = "3", width=2,  command = lambda:setZoom("3"))   
bz3.grid(row = 5, column = 2, columnspan = colspan  ) 
bz4 = Button(root, text = "4", width=2,  command = lambda:setZoom("4"))   
bz4.grid(row = 5, column = 3, columnspan = colspan  ) 
bz5 = Button(root, text = "5", width=2,  command = lambda:setZoom("5"))   
bz5.grid(row = 5, column = 4, columnspan = colspan  ) 
bz6 = Button(root, text = "6", width=2,  command = lambda:setZoom("6"))   
bz6.grid(row = 5, column = 5, columnspan = colspan  ) 
bz7 = Button(root, text = "7", width=2,  command = lambda:setZoom("7"))   
bz7.grid(row = 5, column = 6, columnspan = colspan  ) 

#delay with grid

bd1 = Button(root, text = "1", width=2, command = lambda:setDelay("1000")) 
bd1.grid(row = 10, column = 0, columnspan = 1)    
bd2 = Button(root, text = "2", width=2, command = lambda:setDelay("2000"))  
bd2.grid(row = 10, column = 1, columnspan = 1)
bd3 = Button(root, text = "3", width=2, command = lambda:setDelay("3000"))  
bd3.grid(row = 10, column = 2, columnspan = 1)
bd4 = Button(root, text = "4", width=2,  command = lambda:setDelay("4000"))  
bd4.grid(row = 10, column = 3, columnspan = 1)
bd5 = Button(root, text = "5", width=2,  command = lambda:setDelay("5000"))  
bd5.grid(row = 10, column = 4, columnspan = 1)
bd6 = Button(root, text = "6", width=2,  command = lambda:setDelay("6000"))  
bd6.grid(row = 10, column = 5, columnspan = 1)
bd7 = Button(root, text = "7", width=2,  command = lambda:setDelay("7000"))  
bd7.grid(row = 10, column = 6, columnspan = 1)
bd8 = Button(root, text = "8", width=2,  command = lambda:setDelay("8000"))  
bd8.grid(row = 10, column = 7, columnspan = 1)
bd9 = Button(root, text = "9", width=2,  command = lambda:setDelay("9000"))  
bd9.grid(row = 10, column = 8, columnspan = 1)
bd10 = Button(root, text = "10", width=2,  command = lambda:setDelay("10000"))  
bd10.grid(row = 10, column = 9, columnspan = 1)

#packing/grid

posLabl.grid(row = 1, column = 0, columnspan = 80)
butStart.grid(row = 2, column = 4, columnspan = 2)
butStop.grid(row = 2, column = 6, columnspan = 2)
zoomLabl.grid(row = 3, column = 0, columnspan = 80)
curntZoomLabl.grid(row = 4, column = 0, columnspan = 80)
delayLabl.grid(row = 6, column = 0, columnspan = 80)
curntDelayLabl.grid(row = 7, column = 0, columnspan = 80)
mapDirLabl.grid(row = 11, column = 0, columnspan = 55)
mapDirBut.grid(row = 12, column = 4, columnspan = 4)
mapDirStat.grid(row= 13, column = 0, columnspan = 7)
mapDirEntry.grid(row = 14, column = 0, columnspan = 25)

#mainloop
root.mainloop()

#close files
log.close()

Answer 1

Is my code sloppy?

It is, but there are very specifics things you can do to address that. Good news! First, use http://pep8online.com/ to make sure that your code follows PEP 8, which is a standard that most Python coders follow to make sure that every Python code can be easily understood by every Python coder.

from Tkinter import *
import time
from datetime import datetime

You need two spaces after the module imports.

#init variables
global r
r = 0
global delay
delay = 1000
#for testing
global lat
lat = "41.5345"
global lon
lon = "-85.343"
global zoom
zoom = 0
global mapDir
mapDir = ""
global mapDirEnabl
mapDirEnabl = 0

Please don't use global variables, you don't need them and they only clutter your code. The good thing about a function is that I can understand it by only looking at it. With global variables, it's not true anymore and I need to be aware of all those variables.

#clear log file
log = open("xpyMapLog.txt", "w")
log.write("")
log.close()
global log
log = open("xpyMapLog.txt", "a")
logStartMessage = "RUN TIME EXECUTED AT: %s \n" % str(datetime.now())
log.write(logStartMessage)

Python has a powerful logging module that you could use. To clear the log file, simply open it in 'w' mode.

#error check
class safe:
    def __init__(self, function):
        self.function = function

    def __call__(self, *args):
        try:
            return self.function(*args)
        except Exception, e:
            errorMsg = "Error: %r" % str(e)
            log.write(errorMsg)  
            print "Error: %s" % e

Make sure that you know why and when you're using the safe decorator. It's a bad idea most of the time: most exceptions are not recoverable and you should instead 1/ make sure they do not happen 2/ or recover when you know how to recover.

        #
    #
#

Remove those, and either get used to Python's indentation or find an editor that helps you to visualize the scope of your blocks.

#open data file
@safe
def openData():
    global Data
    Data = open("Data.txt", "r")
#
openData()

Why is this a function? You only need Data = open("Data.txt", "r") and nothing else if it's not a function. And you won't need the nasty global. @safe also makes no sense here. What do you want to do when opening the data file fails?

#init root window
root = Tk()
root.geometry("550x300+200+200")
root.title("XPYmap for x-plane 10")

Note that Tkinter is not considered a very nice Python library anymore, but I don't know what is the recommended one nowadays. Please rewrite your code considering what I have said already, and maybe resubmit the result on Code Review when it's done.

Answer 2

On top of @Quentin's comments, please try to avoid repeting yourself too much and declaring useless variables as it tends to make things harder to read.

bz1 = Button(root, text = "1", width=2, command = lambda:setZoom("1"))
bz1.grid(row = 5, column = 0, columnspan = colspan)
bz2 = Button(root, text = "2", width=2, command = lambda:setZoom("2"))
bz2.grid(row = 5, column = 1, columnspan = colspan  )
bz3 = Button(root, text = "3", width=2,  command = lambda:setZoom("3"))
bz3.grid(row = 5, column = 2, columnspan = colspan  )
bz4 = Button(root, text = "4", width=2,  command = lambda:setZoom("4"))
bz4.grid(row = 5, column = 3, columnspan = colspan  )
bz5 = Button(root, text = "5", width=2,  command = lambda:setZoom("5"))
bz5.grid(row = 5, column = 4, columnspan = colspan  )
bz6 = Button(root, text = "6", width=2,  command = lambda:setZoom("6"))
bz6.grid(row = 5, column = 5, columnspan = colspan  )
bz7 = Button(root, text = "7", width=2,  command = lambda:setZoom("7"))
bz7.grid(row = 5, column = 6, columnspan = colspan  )

bd1 = Button(root, text = "1", width=2, command = lambda:setDelay("1000"))
bd1.grid(row = 10, column = 0, columnspan = 1)
bd2 = Button(root, text = "2", width=2, command = lambda:setDelay("2000"))
bd2.grid(row = 10, column = 1, columnspan = 1)
bd3 = Button(root, text = "3", width=2, command = lambda:setDelay("3000"))
bd3.grid(row = 10, column = 2, columnspan = 1)
bd4 = Button(root, text = "4", width=2,  command = lambda:setDelay("4000"))
bd4.grid(row = 10, column = 3, columnspan = 1)
bd5 = Button(root, text = "5", width=2,  command = lambda:setDelay("5000"))
bd5.grid(row = 10, column = 4, columnspan = 1)
bd6 = Button(root, text = "6", width=2,  command = lambda:setDelay("6000"))
bd6.grid(row = 10, column = 5, columnspan = 1)
bd7 = Button(root, text = "7", width=2,  command = lambda:setDelay("7000"))
bd7.grid(row = 10, column = 6, columnspan = 1)
bd8 = Button(root, text = "8", width=2,  command = lambda:setDelay("8000"))
bd8.grid(row = 10, column = 7, columnspan = 1)
bd9 = Button(root, text = "9", width=2,  command = lambda:setDelay("9000"))
bd9.grid(row = 10, column = 8, columnspan = 1)
bd10 = Button(root, text = "10", width=2,  command = lambda:setDelay("10000"))
bd10.grid(row = 10, column = 9, columnspan = 1)

can be rewritten :

for i in range(7):
    txt = str(i+1)
    but = Button(root, text = txt, width=2, command = lambda:setZoom(txt))
    but.grid(row = 5, column = i, columnspan = colspan)

for i in range(10):
    txt = str(i+1)
    delay = str((i+1)*1000)
    but = Button(root, text = txt,  width=2, command = lambda:setDelay(delay))
    but.grid(row = 10, column = i, columnspan = 1)