Paper label system in Python

Question

Update from Digitizing paper label system with Python

I have implemented a username and password log in and main application to be used after successful log in.

I am ready for some more feedback.

My main troubles is error handling. For example log in screen, I'll need to implement statement to show if user does not exist etc.. Nothing too fancy. Also, maybe add a scroll bar and center the screen for both classes when opened up.

Also, if there is nothing inside a folder, the drop down menu dissapears and shows nothing, I'll need to work on that too. Maybe pop another screen saying 'No files in folder, come back later'.

Here is my full code:

from tkinter import *
from tkinter import DISABLED, messagebox
import tkinter.ttk as ttk
import os
import glob
from PIL import Image, ImageTk, ImageGrab
from pathlib import Path
import pyautogui
import time

def main():
    root = Tk()
    app = Window1(root)
    root.mainloop()

class Window1:
    def __init__(self,master):
        self.master = master
        self.master.title("User Log In")
        self.master.geometry('400x150')
        self.frame = Frame(self.master)
        self.frame.pack(fill="both", expand=True)


        self.label_username = Label(self.frame, text="Username: ",font=("bold",16))
        self.entry_username = Entry(self.frame, font = ("bold", 14))
        self.label_password = Label(self.frame, text="Password: ",font=("bold",16))
        self.entry_password = Entry(self.frame, show="*", font = ("bold", 14))

        self.label_username.pack()
        self.entry_username.pack()
        self.label_password.pack()
        self.entry_password.pack()

        self.logbtn = Button(self.frame, text="Login", font = ("bold", 10), command=self._login_btn_clicked)
        self.logbtn.pack()

    #close and stop tkinter running in backround, also see line #64
    def on_closing(self):
        self.master.destroy()


    def _login_btn_clicked(self):
        # print("Clicked")
        username = self.entry_username.get()
        password = self.entry_password.get()

        # print(username, password)
        account_list = [line.split(":", maxsplit=1) for line in open("passwords.txt")]
        # list of 2-tuples. Usersnames with colons inside not supported.
        accounts = {key: value.rstrip() for key, value in account_list}
        # Convert to dict[username] = password, and slices off the line ending.
        # Does not support passwords ending in whitespace.
        if accounts[username] == password:
            self.master.withdraw()
            self.newWindow = Toplevel(self.master)
            self.newWindow.protocol("WM_DELETE_WINDOW", self.on_closing)
            self.app = Window2(self.newWindow, window1 = self)
        else:
            messagebox.showinfo("User message", "Invalid username or password specified please try again")


class Window2:
    def __init__(self,master, window1):
        notebook = ttk.Notebook(master)

        notebook.pack(expand = 1, fill = "both")
        #Frames
        main = ttk.Frame(notebook)
        manual = ttk.Frame(notebook)
        notebook.add(main, text='Main-Screen')
        notebook.add(manual, text='Manual')
        self.window1 = window1


        def clock():
            t=time.strftime('%d/%m/%Y, %H:%M:%S, ',time.localtime())
            if t!='':
                self.display_time.config(text=t,font='times 15')
            main.after(100,clock)
        self.display_time=Label(main)
        self.display_time.grid(column = 3, row = 0)
        clock()


        username = self.window1.entry_username.get()
        self.User = Label(main, text = 'User: '+ username, font = ('15'))
        self.User.grid(column = 4, row = 0)


        var1 = IntVar()
        var2 = IntVar()
        var3 = IntVar()
        var4 = IntVar()
        var5 = IntVar()
        var6 = IntVar()
        var7 = IntVar()
        var8 = IntVar()
        var9 = IntVar()
        var10 = IntVar()
        var11 = IntVar()
        var12 = IntVar()

        #Displaying checkboxes and assigning to variables

        self.Checkbox1 = Checkbutton(main, text="Ingredients present in full (any allergens in bold with allergen warning if necessary)", variable=var1)
        self.Checkbox1.grid(column = 2, row = 1, sticky = W)

        self.Checkbox2 = Checkbutton(main, text="May Contain Statement.", variable=var2)
        self.Checkbox2.grid(column = 2, row = 2, sticky = W)

        self.Checkbox3 = Checkbutton(main, text="Cocoa Content (%).", variable=var3)
        self.Checkbox3.grid(column = 2, row = 3, sticky = W)

        self.Checkbox4 = Checkbutton(main, text="Vegetable fat in addition to Cocoa butter", variable=var4)
        self.Checkbox4.grid(column = 2, row = 4, sticky = W)

        self.Checkbox5 = Checkbutton(main, text="Instructions for Use.", variable=var5)
        self.Checkbox5.grid(column = 2, row = 5, sticky = W)

        self.Checkbox6 = Checkbutton(main, text="Additional warning statements (pitt/stone, hyperactivity etc)", variable=var6)
        self.Checkbox6.grid(column = 2, row = 6, sticky = W)

        self.Checkbox7 = Checkbutton(main, text="Nutritional Information Visible", variable=var7)
        self.Checkbox7.grid(column = 2, row = 7, sticky = W)

        self.Checkbox8 = Checkbutton(main, text="Storage Conditions", variable=var8)
        self.Checkbox8.grid(column = 2, row = 8, sticky = W)

        self.Checkbox9 = Checkbutton(main, text="Best Before & Batch Information", variable=var9)
        self.Checkbox9.grid(column = 2, row = 9, sticky = W)

        self.Checkbox10 = Checkbutton(main, text="Net Weight & Correct Font Size.", variable=var10)
        self.Checkbox10.grid(column = 2, row = 10, sticky = W)

        self.Checkbox11 = Checkbutton(main, text="Barcode - Inner", variable=var11)
        self.Checkbox11.grid(column = 2, row = 11, sticky = W)

        self.Checkbox12 = Checkbutton(main, text="Address & contact details correct", variable=var12)
        self.Checkbox12.grid(column = 2, row = 12, sticky = W)


        #PrintScreen
        def PrintScreen():
            pyautogui.keyDown('alt')
            pyautogui.keyDown('printscreen')
            pyautogui.keyUp('printscreen')
            pyautogui.keyUp('alt')
            self.dataSend['state'] = 'normal'

        def var_states():
            text_file = open("logfile.txt", "a")
            text_file.write("Username: [%s], option 1: [%d], option 2: [%d], option 3: [%d], option 4: [%d], option 5: [%d], option 6: [%d], option 7: [%d], option 8: [%d], option 9: [%d], option 10: [%d], option 11: [%d], option 12: [%d], Original Sign Off: [%s]\n" % (username,var1.get(),  var2.get(),  var3.get(),  var4.get(),  var5.get(),  var6.get(),  var7.get(),  var8.get(),  var9.get(),  var10.get(),  var11.get(),  var12.get(), self.p))
            text_file.close()
            self.img = ImageGrab.grabclipboard()
            self.img.save("%s" % ('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/'+ os.path.basename(self.p).strip('- to sign.jpg') + ' ' + username+ '.jpg'), 'JPEG')
            ed = ('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/'+ os.path.basename(self.p).strip('- to sign.jpg') +' ed'+ '.jpg')
            Nb = ('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/'+ os.path.basename(self.p).strip('- to sign.jpg') +' Nb'+ '.jpg')
            jj = ('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/'+ os.path.basename(self.p).strip('- to sign.jpg') +' jj'+ '.jpg')
            kl = ('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/'+ os.path.basename(self.p).strip('- to sign.jpg') +' kl'+ '.jpg')

            if os.path.exists(ed) and os.path.exists(Nb) or os.path.exists(jj) or os.path.exists(kl):
                os.remove('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/To sign/' + os.path.basename(self.p))
            else:
                print("False")





        self.dataSend = Button(main, text = "Send", command = var_states, state = DISABLED)
        self.dataSend.grid(column = 1, row = 13, sticky = W)
        self.CaptureScreen = Button(main, text = "PrintScreen", command = PrintScreen, state = DISABLED)
        self.CaptureScreen.grid(column = 1, row = 14, sticky = W)

###################################################################################################################################
                                                        ##Load Image##
###################################################################################################################################
        # Create a Tkinter variable
        self.tkvar = StringVar()

        # Directory
        self.nonedisplayed = "N/A"
        self.directory = "//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/To Sign"
        self.choices = glob.glob(os.path.join(self.directory, "*- to sign.jpg"))



        self.tkvar.set('...To Sign Off...') # set the default option

        # Images
        def change_dropdown():
            imgpath = self.tkvar.get()
            img = Image.open(imgpath)
            photo = ImageTk.PhotoImage(img)
            label2.image = photo
            label2.configure(image=photo)
            self.CaptureScreen['state'] = 'normal'


        #return path value
        self.p = None
        def func(value):
            global p
            self.p = Path(value)
            print(self.p)

        #widgets
        self.msg1 = Label(main, text = "Choose here")
        self.msg1.grid(column = 0, row = 0)
        self.popupMenu = OptionMenu(main, self.tkvar, *self.choices, command = func)
        self.popupMenu.grid(row=1, column=0)
        self.display_label = label2 = Label(main, image=None)
        self.display_label.grid(row=2, column=0, rowspan = 500)
        self.open_button = Button(main, text="Open", command=change_dropdown)
        self.open_button.grid(row=502, column=0)

###################################################################################################################################
                                                        ##TAB 2 - MANUAL##
###################################################################################################################################

        def manualopen():
            os.startfile('//SERVER/shared_data/Technical/Food Safety & Quality Manual/Section 21 - Process Control/21.LABL.02 - Labelling notes.docx')

        self.manualBtn = Button(manual, text= "open doc", command = manualopen)
        self.manualBtn.pack()




if __name__ == '__main__':
    main()

Also, my passwords.txt file looks like this:

ed:password1
Nb:rother
jj:4spaces
kl:timepass

Answer 1

Storing passwords in plaintext

I already mentioned this in a comment to your question: Don't store passwords in plaintext! It's usually not even a good idea for testing (when are you going to test the password storage then?). Plus, it's quite easy to do in Python e.g. using the passlib library. Inspired from their example section:

from passlib.hash import pbkdf2_sha256

hash_ = pbkdf2_sha256.hash("password")
print(hash_)
$pbkdf2-sha256$29000$2RtjbM0ZY8y5l/IeQyhFCA$wxMKXJPS6gtJRKHWki1.z0UQHQlR292ZvFvWEm0wSYc
# verify provides a constant time key comparison function in order to avoid timing attacks
assert pbkdf2_sha256.verify("password", hash_)
assert not pbkdf2_sha256.verify("password2", hash_)

The example above uses PBKDF2 algorithm to store the password, but passlib also supports bcrypt, scrypt, and the current "state of the art" argon2. All of them are hash functions specifically designed to be used for password storage.

If you don't want to work with an external library, the Python standard library module hashlib has hashlib.scrypt from Python 3.6 onwards (if your platform supports OpenSSL). The usage of this function might not be as smooth as with the library above, and will require a little bit more care from your side.

Hard-coded paths

Your application has a few hard-coded paths where your labels should be stored (probably?). You'd usually want to avoid that, so that your code does not have to be changed whenever the location of those files should change. Environment variables or a config file, maybe also written in JSON, are things to look at. If that's all to fancy for what you have in mind, at least store them into a MODULE_LEVEL_CONSTANT in order to avoid copy-and-pasting the same long string over and over again.

Reading and writing files

When using open(...) it's a common best practice to use it in conjunction with a context manager, aka with statement. Using with open(...) as file_: takes care to properly close the file no matter what, i.e. also in the event of an exception, which would usually require using something like try: ... finally: ....

String formatting

The current code uses the "old style" % string formatting, which was superseded by .format(...) in Python 2.7 and f-strings in Python 3.6.

foo = 1
# old style
"%d" % foo
# Python 2.7+
"{}".format(foo)
# Python 3.6+
f"{foo}"

Especially f-string are very powerful and convenient. Maybe have a look at this blog post if you are interested to learn more about each of the three variants and their features.

Style and formatting

The overall appearance of the code seems a bit rough. Especially blank lines are not used very to structure the code into logical blocks. Since your code has grown quite a bit, I'd highly recommend looking for an IDE with autoformatting capabilities or at least a style-checker integration (Flake8, Pylint, ...). This meta post here on Code Review lists a good first overview of tools and editors to choose from.

Answer 2

First lets fix your imports.

You are importing * and you should do import tkinter as tk so you don't overwrite anything. You are also importing DISABLED and that is already imported from * so on the very next line you are already overwriting imports.

Next did major PEP8 clean up. This includes proper spacing between qualifiers, classes, functions, comments and so on as well as working on the max line length for coding.

Next we got rid of a lot of unneeded self prefixes. You only need self when you have a class attribute or method you will need to use again in the code down the line.

Then we need to replace all you concatenation with format() as format is the current correct method of concatenation.

One big save on rows is to build your Check Buttons into a list and then reference that list when writing your data.

One last change was to convert your Window1 class to be the root window through inheritance as well as convert Window2 to a Toplevel class. This allows us to use self.master to work with the parent window.

I got your code down to 147 lines of code and I could probably get it down lower if I could do some testing. Take a look at the below and let me know if you have any questions.

I do have a concern on this line:

account_list = [line.split(":", maxsplit=1) for line in open("passwords.txt")]

I am a fan of one liners however I think (I may be wrong) this remains open as you never tell it to close. I would instead load the file using a with open() statement instead and then take the data and throw that into your one liner. The benefit to using with open() is that once the work is complete it will auto close the connection/file.

I will be adding more detail to this answer when I get home but for now this should be a good start.

Edit: Some more error fixing.

import tkinter as tk
import tkinter.ttk as ttk
from tkinter import messagebox
from PIL import Image, ImageTk, ImageGrab
from pathlib import Path
import pyautogui
import glob
import time
import os


def main():
    Window1().mainloop()


def manual_open():
    os.startfile('//SERVER/shared_data/Technical/Food Safety & Quality Manual/Section 21 -'
                 ' Process Control/21.LABL.02 - Labelling notes.docx')


class Window1(tk.Tk):
    def __init__(self):
        super().__init__()
        self.title("User Log In")
        self.geometry('400x150')
        frame = tk.Frame(self)
        frame.pack(fill="both", expand=True)

        tk.Label(frame, text="Username: ", font=("bold", 16)).pack()
        self.entry_username = tk.Entry(frame, font=("bold", 14))
        self.entry_username.pack()
        tk.Label(frame, text="Password: ", font=("bold", 16)).pack()
        self.entry_password = tk.Entry(frame, show="*", font=("bold", 14))
        self.entry_password.pack()
        tk.Button(frame, text="Login", font=("bold", 10), command=self._login_btn_clicked).pack()

    def on_closing(self):
        self.destroy()

    def _login_btn_clicked(self):
        username = self.entry_username.get()
        password = self.entry_password.get()
        account_list = [line.split(":", maxsplit=1) for line in open("passwords.txt")]
        accounts = {key: value.rstrip() for key, value in account_list}
        if accounts[username] == password:
            self.withdraw()
            self.app = Window2()
            self.app.protocol("WM_DELETE_WINDOW", self.on_closing)
        else:
            messagebox.showinfo("User message", "Invalid username or password specified please try again")


class Window2(tk.Toplevel):
    def __init__(self):
        super().__init__()
        notebook = ttk.Notebook(self)
        notebook.pack(expand=1, fill="both")
        main = ttk.Frame(notebook)
        manual = ttk.Frame(notebook)
        notebook.add(main, text='Main-Screen')
        notebook.add(manual, text='Manual')
        self.display_time = tk.Label(main)
        self.display_time.grid(column=3, row=0)
        self.clock()
        self.username = self.master.entry_username.get()
        tk.Label(main, text='User: {}'.format(self.username), font='15').grid(column=4, row=0)

        self.checkbutton_list = []
        checkbutton_verb = ["Ingredients present in full (any allergens in bold with allergen warning if necessary)",
                            "May Contain Statement.", "Cocoa Content (%).", "Vegetable fat in addition to Cocoa butter",
                            "Instructions for Use.", "Additional warning statements (pitt/stone, hyperactivity etc)",
                            "Nutritional Information Visible", "Storage Conditions", "Best Before & Batch Information",
                            "Net Weight & Correct Font Size.", "Barcode - Inner", "Address & contact details correct"]

        for ndex, i in enumerate(checkbutton_verb):
            x = tk.IntVar()
            self.checkbutton_list.append([tk.Checkbutton(main, text=i, variable=x), x])
            self.checkbutton_list[-1][0].grid(column=2, row=ndex+1, sticky='w')

        directory = "//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/To Sign"
        choices = glob.glob(os.path.join(directory, "*- to sign.jpg"))
        tk.Label(main, text="Choose here").grid(column=0, row=0)
        self.tkvar = tk.StringVar()
        self.tkvar.set('...To Sign Off...')
        tk.OptionMenu(main, self.tkvar, *choices, command=self.func).grid(row=1, column=0)
        self.label2 = tk.Label(main, image=None)
        self.label2.grid(row=2, column=0, rowspan=500)
        tk.Button(main, text="Open", command=self.change_dropdown).grid(row=502, column=0)
        self.dataSend = tk.Button(main, text="Send", command=self.var_states, state='disabled')
        self.dataSend.grid(column=1, row=13, sticky='w')
        self.CaptureScreen = tk.Button(main, text="PrintScreen", command=self.print_screen, state='disabled')
        self.CaptureScreen.grid(column=1, row=14, sticky='w')
        self.manualBtn = tk.Button(manual, text="open doc", command=manual_open)
        self.manualBtn.pack()

    def clock(self):
        t = time.strftime('%d/%m/%Y, %H:%M:%S, ', time.localtime())
        if t != '':
            self.display_time.config(text=t, font='times 15')
        self.after(1000, self.clock)

    def print_screen(self):
        pyautogui.keyDown('alt')
        pyautogui.keyDown('printscreen')
        pyautogui.keyUp('printscreen')
        pyautogui.keyUp('alt')
        self.dataSend['state'] = 'normal'

    def var_states(self):
        text_file = open("logfile.txt", "a")
        formatted_string = 'Username: {}'.format(self.username)
        for ndex, sub_list in enumerate(self.checkbutton_list):
            formatted_string = '{}, Option {}: '.format(formatted_string, ndex+1, sub_list[1].get())
        text_file.write(formatted_string)
        text_file.close()
        self.img = ImageGrab.grabclipboard()
        self.img.save('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/{} {} .jpg'
                      .format(os.path.basename(self.p).strip('- to sign.jpg'), self.username), 'JPEG')
        ed = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/{} ed.jpg'\
             .format(os.path.basename(self.p).strip('- to sign.jpg'))
        nb = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/{} Nb.jpg'\
             .format(os.path.basename(self.p).strip('- to sign.jpg'))
        jj = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/{} jj.jpg'\
             .format(os.path.basename(self.p).strip('- to sign.jpg'))
        kl = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/{} kl.jpg'\
             .format(os.path.basename(self.p).strip('- to sign.jpg'))

        if os.path.exists(ed) and os.path.exists(nb) or os.path.exists(jj) or os.path.exists(kl):
            os.remove('//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/To sign/{}'
                      .format(os.path.basename(self.p)))
        else:
            print("False")

    def change_dropdown(self):
        img = Image.open(self.tkvar.get())
        photo = ImageTk.PhotoImage(img)
        self.label2.image = photo
        self.label2.configure(image=photo)
        self.CaptureScreen['state'] = 'normal'
        self.p = None

    def func(self, value):
        self.p = Path(value)
        print(self.p)


if __name__ == '__main__':
    main()

For your error in the comments I am not getting this same issue. Please provide full trace back. As you can see below I can print the value of accounts resulting from this line accounts = {key: value.rstrip() for key, value in account_list} and that is the same line you get an error on.