7
\$\begingroup\$

Background

My lab administers four computer programs in three separate appointments. At each appointment, the subject does the programs in a pseudo-randomized order. Also, three of the programs have subversions, which are also pseudo-randomized so that the subject could do type A of program 1 at the first appointment, type C of program 1 at followup 1, and type B of program 1 at followup 2.

The orders and individual program subversions that subjects have run in the past are stored in csv files (one file for each program, and one file for overall program order). The appropriate pseudo-randomized order for each timepoint is stored as a list of lists in a pickle file.

Code Purpose

Basically, my GUI takes in the unique subject ID number and appointment number and looks through the overall order file to determine if the subject has been recorded, and determines, based on the row in which the subject ID occurs (a new row if the subject is new), which order the programs should be given in and which version of each program should be performed. It also updates the csv with that information. Once the GUI determines the appropriate program and subversion to run, it simply opens up the program, waits until the subject has finished with the task, and then executes the next program.

It is supposed to deal with input errors, like overwriting or inputting an unacceptable appointment number, by giving the option to restart or quit gracefully. Ultimately, I will create a shortcut for this script on a Windows computer so that it can be run purely graphically.

Review Goals

I am hoping to make my code as efficient and readable as possible, so any advice is welcomed, but I do have a couple of specific concerns:

  1. How can I clean up get_curr_order's inputs? Can I make set_overwrite a global variable?

  2. Is there a better way to handle restarting the GUI in get_curr_order?

  3. Is there a better way to handle exceptions in my try/except statements? Both exceptions are perfectly normal cases, so I don't want it to break, but pylint doesn't like except Exception or just except.

Below are the code itself and a separate section with the lists of lists and dictionaries that I pickled for the code.

import Tkinter
import csv
import pickle
import sys
import psutil
import os
import subprocess


class GUI(Tkinter.Tk):
    """
    Main graphical user interface for behav_gui. Used to input subject ID and
    timepoint.
    """
    def __init__(self):
        Tkinter.Tk.__init__(self)
        self.wm_title("GUI")

        label_1 = Tkinter.Label(self, text="Subject ID")
        label_1.grid(row=1, column=1)
        self.sub_input = Tkinter.StringVar()
        entry_1 = Tkinter.Entry(self, bd=5, textvariable=self.sub_input)
        entry_1.grid(row=1, column=2)

        label_2 = Tkinter.Label(self, text="Timepoint")
        label_2.grid(row=2, column=1)
        self.tp_input = Tkinter.IntVar()
        entry_2 = Tkinter.Entry(self, bd=5, textvariable=self.tp_input)
        entry_2.grid(row=2, column=2)

        label_3 = Tkinter.Label(self, text="BL: 1, 12: 3, 24: 4")
        label_3.grid(row=3, column=2)

        label_4 = Tkinter.Label(self, text="")
        label_4.grid(row=4, column=2)

        label_5 = Tkinter.Button(self, text="Done", command=self.close_window)
        label_5.grid(row=5, column=1, columnspan=2)

#        self.bind("<Return>", self.close_window)

        self.mainloop()

    def close_window(self, *args):
        """ Closes the GUI window."""
        del args
        self.destroy()


class OverwriteButton(Tkinter.Tk):
    """
    Creates a window with query (Do you wish to overwrite) and two buttons, Yes
    and No. Each sets response attribute to respective string.
    """
    def __init__(self):
        Tkinter.Tk.__init__(self)
        self.wm_title("Duplicate")
        self.response = "No"

        label_1 = Tkinter.Label(self,
                                text="Duplicate ID and TP given. Overwrite?")
        label_1.grid(row=1, column=1, columnspan=5)

        button_1 = Tkinter.Button(self, text="Yes", command=self.respond_yes)
        button_1.grid(row=2, column=2, columnspan=1)

        button_2 = Tkinter.Button(self, text="No", command=self.respond_no)
        button_2.grid(row=2, column=4, columnspan=1)

        self.mainloop()

    def respond_yes(self):
        """ Closes the GUI window and sets response to Yes."""
        self.response = "Yes"
        self.destroy()

    def respond_no(self):
        """ Closes the GUI window and sets response to No."""
        self.response = "No"
        self.destroy()


class ContinueButton(Tkinter.Tk):
    """
    Creates a window with inputted label and three buttons, Continue, Restart,
    and Quit. Each sets response attribute to respective string.
    """
    def __init__(self, label):
        Tkinter.Tk.__init__(self)
        self.wm_title("Continue")
        self.response = "Quit"

        label_1 = Tkinter.Label(self, text=label)
        label_1.grid(row=1, column=1, columnspan=5)

        label_2 = Tkinter.Label(self, text="Do you wish to move on?")
        label_2.grid(row=2, column=1, columnspan=5)

        button_1 = Tkinter.Button(self, text="Continue",
                                  command=self.continue_)
        button_1.grid(row=3, column=1, columnspan=1)

        button_2 = Tkinter.Button(self, text="Restart", command=self.restart)
        button_2.grid(row=3, column=3, columnspan=1)

        button_3 = Tkinter.Button(self, text="Quit", command=self.close_window)
        button_3.grid(row=3, column=5, columnspan=1)

        self.mainloop()

    def continue_(self):
        """ Closes the GUI window."""
        self.response = "Continue"
        self.destroy()

    def restart(self):
        """ Closes the GUI window."""
        self.response = "Restart"
        self.destroy()

    def close_window(self):
        """ Closes the GUI window."""
        self.response = "Quit"
        self.destroy()


class RetryButton(Tkinter.Tk):
    """
    Creates a window with one inputted label and two buttons- Restart and Quit.
    Each sets response attribute to respective string.
    """
    def __init__(self, label):
        Tkinter.Tk.__init__(self)
        self.wm_title("Problem")
        self.response = "Restart"

        label_1 = Tkinter.Label(self, text=label)
        label_1.grid(row=1, column=1, columnspan=5)

        button_1 = Tkinter.Button(self, text="Restart", command=self.restart)
        button_1.grid(row=2, column=1, columnspan=1)

        button_2 = Tkinter.Button(self, text="Quit", command=self.quit_)
        button_2.grid(row=2, column=5, columnspan=1)

        self.mainloop()

    def restart(self):
        """ Closes the GUI window and sets response to Restart."""
        self.response = "Restart"
        self.destroy()

    def quit_(self):
        """ Closes the GUI window and sets response to Quit."""
        self.response = "Quit"
        self.destroy()


class LabelMaker(Tkinter.Tk):
    """
    Creates a window with an inputted title and label and a close button
    labeled "Okay".
    """
    def __init__(self, title, label):
        Tkinter.Tk.__init__(self)
        self.wm_title(title)
        label_1 = Tkinter.Label(self, text=label)
        label_1.grid(row=1, column=1, columnspan=5)

        button_1 = Tkinter.Button(self, text="Okay", command=self.close_window)
        button_1.grid(row=2, column=3, columnspan=1)

        self.mainloop()

    def close_window(self):
        """ Closes the GUI window."""
        self.destroy()


def get_curr_order(task_order_csv, task_order, subject_id, subject_tp, tp_dict,
                   col_beg, col_end, set_overwrite, all_tasks):
    """
    Takes subject ID, timepoints, organization of task counterbalancing, and
    current list of task counterbalances (from csv) and returns correct order
    for given subject and timepoint, as well as updates list of lists (from
    csv). How can I clean up inputs? There are too many already and I'm
    considering one more.
    """
    with open(task_order_csv, 'r') as file_:
        task_file = list(csv.reader(file_, delimiter=','))

    subjects = [row[0] for row in task_file]

    if subject_tp in tp_dict.keys():
        curr_order_list = task_order[tp_dict.get(subject_tp)]
    else:
        top = RetryButton(str(subject_tp) + " is not an acceptable timepoint.")
        if top.response == "Restart":  # <-- Is there a cleaner way to do this?
            run_script()
            sys.exit()
        else:
            sys.exit()

    # Find row corresponding to subject in csv. If subject is new, append an
    # empty row to fill in.
    try:
        subject_pos = subjects.index(subject_id)
    except Exception:  # <-- Which exception can I use to pass without error?
        subject_pos = len(subjects)
        task_file.append([""] * len(task_file[0]))

    # If correct position in spreadsheet is empty, fill in with correct list.
    # Else, offer option to overwrite or quit.
    curr_order = curr_order_list[(subject_pos-1) % len(curr_order_list)]
    if not task_file[subject_pos][col_beg[tp_dict.get(subject_tp)]]:
        task_file[subject_pos][0] = subject_id
        task_file[subject_pos][col_beg[tp_dict.get(subject_tp)]:
                               col_end[tp_dict.get(subject_tp)]] = curr_order
        if all_tasks is True:
            LabelMaker("Order", "The current order is: " +
                       ", ".join(curr_order))
        return curr_order, task_file, set_overwrite
    else:
        if set_overwrite == 0:
            top = OverwriteButton()
            if top.response == "Yes":
                set_overwrite = 1
                if all_tasks is True:
                    LabelMaker("Order", "The current order is: " +
                               ", ".join(curr_order))
                return curr_order, task_file, set_overwrite
            else:
                sys.exit()
        else:
            return curr_order, task_file, set_overwrite


def safe_name(process):
    """
    Check for names of processes in psutil.process_iter and, if
    permission denied, returns "None".
    """
    try:
        return process.name
    except Exception:  # <-- Which exception can I use to pass without error?
        return "None"


def execute_file(run_file):
    """
    Opens E-Run (or other specified) file and waits for E-Run to no longer be
    in current processes before continuing.
    """
    if os.name == 'nt':
        subprocess.Popen(run_file, shell=True)
    elif os.name == 'posix':
        subprocess.call(('xdg-open', run_file))
    open_proc = True

    while open_proc is True:
        data = list(psutil.process_iter())
        if any(["gedit" in safe_name(Proc) for Proc in data]):
            open_proc = True
        else:
            open_proc = False


def run_script():
    """
    Runs full script (opens GUI windows, updates csvs, and opens E-Run files).
    """
    task_order_csv = "/home/code/behav_gui/task_order.csv"
    set_overwrite = 0
    input_window = GUI()
    subject_id = input_window.sub_input.get()
    subject_tp = input_window.tp_input.get()

    with open('/home/code/behav_gui/task_order.pickle') as file_:
        [task_order, tp_dict, col_beg, col_end] = pickle.load(file_)

    with open('/home/code/behav_gui/each_order.pickle') as file_:
        each_order = pickle.load(file_)

    with open('/home/code/behav_gui/task_info.pickle') as file_:
        task_info = pickle.load(file_)

    with open('/home/code/behav_gui/file_dict.pickle') as file_:
        file_dict = pickle.load(file_)

    all_tasks = True
    curr_order, task_file, set_overwrite = get_curr_order(task_order_csv,
                                                          task_order,
                                                          subject_id,
                                                          subject_tp,
                                                          tp_dict,
                                                          col_beg,
                                                          col_end,
                                                          set_overwrite,
                                                          all_tasks)
    with open(task_order_csv, 'w') as file_:
        file_ = csv.writer(file_)
        for row in task_file:
            file_.writerow(row)

    # Loop through tasks, reading csv/getting current task type for each and
    # adding to lists of lists ind_ord (task order or type), ind_file (read-in
    # csv as list of lists), and run_file (E-Run file corresponding to specific
    # task order or type).
    ind_ord = [[] for i in curr_order]
    ind_file = [[] for i in curr_order]
    run_file = [[] for i in curr_order]
    all_tasks = False

    for a, task in enumerate(curr_order):
        task_order_csv = task_info.get(task).get("file")
        task_order = each_order.get(task)
        col_beg = task_info.get(task).get("col_beg")
        col_end = task_info.get(task).get("col_end")
        ind_ord[a], ind_file[a], set_overwrite = get_curr_order(task_order_csv,
                                                                task_order,
                                                                subject_id,
                                                                subject_tp,
                                                                tp_dict,
                                                                col_beg,
                                                                col_end,
                                                                set_overwrite,
                                                                all_tasks)
        with open(task_order_csv, 'w') as file_:
            file_ = csv.writer(file_)
            for row in ind_file[a]:
                file_.writerow(row)

        run_file[a] = file_dict.get(task).get(ind_ord[a][0])

    # Loop through tasks and execute files in order. Ask to continue after each
    # task finishes.
    for iTask in range(len(run_file)):
        response = "Restart"
        while response == "Restart":
            execute_file(run_file[iTask])

            # When run_file is closed, move on to next.
            cont = ContinueButton(curr_order[iTask] + " is complete.")
            response = cont.response

            if response == "Quit":
                sys.exit()

    LabelMaker("Congrats!", "Congratulations. You're done.")


if __name__ == "__main__":
    run_script()

And the pickled variables:

import pickle


task_order = [[['AX', 'RISE', 'Kirby', 'Decimal'],
               ['AX', 'RISE', 'Decimal', 'Kirby'],
               ['AX', 'Kirby', 'RISE', 'Decimal'],
               ['AX', 'Kirby', 'Decimal', 'RISE'],
               ['AX', 'Decimal', 'RISE', 'Kirby'],
               ['AX', 'Decimal', 'Kirby', 'RISE'],
               ['RISE', 'AX', 'Kirby', 'Decimal'],
               ['RISE', 'AX', 'Decimal', 'Kirby'],
               ['RISE', 'Kirby', 'AX', 'Decimal'],
               ['RISE', 'Kirby', 'Decimal', 'AX'],
               ['RISE', 'Decimal', 'AX', 'Kirby'],
               ['RISE', 'Decimal', 'Kirby', 'AX'],
               ['Kirby', 'AX', 'RISE', 'Decimal'],
               ['Kirby', 'AX', 'Decimal', 'RISE'],
               ['Kirby', 'RISE', 'AX', 'Decimal'],
               ['Kirby', 'RISE', 'Decimal', 'AX'],
               ['Kirby', 'Decimal', 'AX', 'RISE'],
               ['Kirby', 'Decimal', 'RISE', 'AX'],
               ['Decimal', 'AX', 'RISE', 'Kirby'],
               ['Decimal', 'AX', 'Kirby', 'RISE'],
               ['Decimal', 'RISE', 'AX', 'Kirby'],
               ['Decimal', 'RISE', 'Kirby', 'AX'],
               ['Decimal', 'Kirby', 'AX', 'RISE'],
               ['Decimal', 'Kirby', 'RISE', 'AX']],
              [['RISE', 'AX', 'Kirby', 'Decimal'],
               ['RISE', 'AX', 'Decimal', 'Kirby'],
               ['RISE', 'Kirby', 'AX', 'Decimal'],
               ['RISE', 'Kirby', 'Decimal', 'AX'],
               ['RISE', 'Decimal', 'AX', 'Kirby'],
               ['RISE', 'Decimal', 'Kirby', 'AX'],
               ['Decimal', 'RISE', 'AX', 'Kirby'],
               ['Decimal', 'RISE', 'Kirby', 'AX'],
               ['Decimal', 'AX', 'RISE', 'Kirby'],
               ['Decimal', 'AX', 'Kirby', 'RISE'],
               ['Decimal', 'Kirby', 'RISE', 'AX'],
               ['Decimal', 'Kirby', 'AX', 'RISE'],
               ['AX', 'RISE', 'Kirby', 'Decimal'],
               ['AX', 'RISE', 'Decimal', 'Kirby'],
               ['AX', 'Kirby', 'RISE', 'Decimal'],
               ['AX', 'Kirby', 'Decimal', 'RISE'],
               ['AX', 'Decimal', 'RISE', 'Kirby'],
               ['AX', 'Decimal', 'Kirby', 'RISE'],
               ['Kirby', 'RISE', 'AX', 'Decimal'],
               ['Kirby', 'RISE', 'Decimal', 'AX'],
               ['Kirby', 'AX', 'RISE', 'Decimal'],
               ['Kirby', 'AX', 'Decimal', 'RISE'],
               ['Kirby', 'Decimal', 'RISE', 'AX'],
               ['Kirby', 'Decimal', 'AX', 'RISE']],
              [['Kirby', 'AX', 'RISE', 'Decimal'],
               ['Kirby', 'AX', 'Decimal', 'RISE'],
               ['Kirby', 'RISE', 'AX', 'Decimal'],
               ['Kirby', 'RISE', 'Decimal', 'AX'],
               ['Kirby', 'Decimal', 'AX', 'RISE'],
               ['Kirby', 'Decimal', 'RISE', 'AX'],
               ['Decimal', 'Kirby', 'AX', 'RISE'],
               ['Decimal', 'Kirby', 'RISE', 'AX'],
               ['Decimal', 'AX', 'Kirby', 'RISE'],
               ['Decimal', 'AX', 'RISE', 'Kirby'],
               ['Decimal', 'RISE', 'Kirby', 'AX'],
               ['Decimal', 'RISE', 'AX', 'Kirby'],
               ['AX', 'Kirby', 'RISE', 'Decimal'],
               ['AX', 'Kirby', 'Decimal', 'RISE'],
               ['AX', 'RISE', 'Kirby', 'Decimal'],
               ['AX', 'RISE', 'Decimal', 'Kirby'],
               ['AX', 'Decimal', 'Kirby', 'RISE'],
               ['AX', 'Decimal', 'RISE', 'Kirby'],
               ['RISE', 'Kirby', 'AX', 'Decimal'],
               ['RISE', 'Kirby', 'Decimal', 'AX'],
               ['RISE', 'AX', 'Kirby', 'Decimal'],
               ['RISE', 'AX', 'Decimal', 'Kirby'],
               ['RISE', 'Decimal', 'Kirby', 'AX'],
               ['RISE', 'Decimal', 'AX', 'Kirby']]]

tp_dict = {1: 0,
           3: 1,
           4: 2,
           }

col_beg = [1, 5, 9]
col_end = [5, 9, 13]

with open('/home/code/behav_gui/task_order.pickle', 'w') as fo:
    pickle.dump([task_order, tp_dict, col_beg, col_end], fo)

each_order = {"RISE": [[['A'], ['A'], ['B'], ['B'], ['C'], ['C']],
                       [['B'], ['C'], ['A'], ['C'], ['A'], ['B']],
                       [['C'], ['B'], ['C'], ['A'], ['B'], ['A']]],
              "AX": [[['1']],
                     [['1']],
                     [['1']]],
              "Kirby": [[['Messy first'], ['Rounded first']],
                        [['Rounded first'], ['Messy first']],
                        [['Messy first'], ['Rounded first']]],
              "Decimal": [[['Messy first'], ['Rounded first']],
                          [['Rounded first'], ['Messy first']],
                          [['Messy first'], ['Rounded first']]],
              }

with open('/home/code/behav_gui/each_order.pickle', 'w') as fo:
    pickle.dump(each_order, fo)

task_info = {"RISE": {"file": "/home/code/behav_gui/r_trialsheet.csv",
                      "col_beg": [1, 2, 3],
                      "col_end": [2, 3, 4],
                      },
             "AX": {"file": "/home/code/behav_gui/a_trialsheet.csv",
                    "col_beg": [1, 2, 3],
                    "col_end": [2, 3, 4]},
             "Kirby": {"file": "/home/code/behav_gui/k_trialsheet.csv",
                       "col_beg": [1, 2, 3],
                       "col_end": [2, 3, 4]},
             "Decimal": {"file": "/home/code/behav_gui/d_trialsheet.csv",
                         "col_beg": [1, 2, 3],
                         "col_end": [2, 3, 4]},
             }

with open('/home/code/behav_gui/task_info.pickle', 'w') as fo:
    pickle.dump(task_info, fo)

file_dict = {"RISE": {"A": "/home/code/behav_gui/a.txt",
                      "B": "/home/code/behav_gui/b.txt",
                      "C": "/home/code/behav_gui/c.txt",
                      },
             "AX": {"1": "/home/code/behav_gui/ax.txt"},
             "Kirby": {"Messy first": "/home/code/behav_gui/mfk.txt",
                       "Rounded first": "/home/code/behav_gui/rfk.txt",
                       },
             "Decimal": {"Messy first": "/home/code/behav_gui/mfd.txt",
                         "Rounded first": "/home/code/behav_gui/rfd.txt",
                         },
             }

with open('/home/code/behav_gui/file_dict.pickle', 'w') as fo:
    pickle.dump(file_dict, fo)
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

UI Code

The windows set a response variable as a string. However, in most cases, the response can only be one of two values. Strings like "Yes" and "No" are good when a UI displays something to a user, but a computer works much better with boolean values.

Just because UI code doesn't mean you no longer have to use good descriptive variable names. Don't use things like label_1 or button_2. Using a number in a variable name is rarely the correct thing to do. 

class OverwriteButton(Tkinter.Tk):
    """
    Creates a window with query (Do you wish to overwrite) and two buttons, Yes
    and No. Each sets response attribute to respective string.
    """

The class name says it is a button, but the documentation says it is a window. Which one is correct?

You recognized that get_curr_order() is taking a lot of parameters. It is acceptable to make a class that just stores data that can be passed into a function.

However, a better idea would be to break the function into smaller sub-functions. The first thing get_curr_order() does is read text from a file. This function should be concerned with the logic of getting the current order, not reading text from a csv file. A bonus benefit of doing this is now you can test this function without having to write the test data to a file before calling the function you want to test.

There are a number of other sub sections within get_curr_order(). If you find yourself writing a comment that says what you are doing, that is a good indication that the following block of code would be a good section that can be extracted into a function with a descriptive name. Comments are better at saying why some operation is being done.

def safe_name(process):
    """
    Check for names of processes in psutil.process_iter and, if
    permission denied, returns "None".
    """
    try:
        return process.name
    except Exception:  # <-- Which exception can I use to pass without error?
        return "None"

Don't catch Exception. You can test the code to see what exception is actually thrown (AttributeError). Better yet, Python has getattr() for getting an attribute and returning a default if it doesn't exist.

while open_proc is True:
    data = list(psutil.process_iter())
    if any(["gedit" in safe_name(Proc) for Proc in data]):
        open_proc = True
    else:
        open_proc = False

You don't have to explicitly use True or False.

while open_proc:
    data = list(psutil.process_iter())
    open_proc = any(["gedit" in safe_name(proc) for proc in data])

Also note the change to proc. CapitalCase is used for classes.

Taking a step back and looking at the code, you should not be using that loop to wait for the process to terminate. call() blocks until the process terminates. [Popen.wait()][2] will do the same.

Pickling Code

At first glance, it looks like task_order includes all of the permutation 'AX', 'RISE', 'Kirby', and 'Decimal'. Python has permutations() for that instead of writing it out. Using the function will be easier to read and less error prone.

In general, the generation of the pickle files is full with lots of constants with not context. It would be a good idea to include some comments that explain what these values are and how they are being used.

All of the paths are hard coded and absolute. It would be better to either write files to locations relative to the working directory (or at least have a variable specifying the base path). Doing this will make it easier for another person to run the code.

Improve this answer
\$\endgroup\$

Your Answer

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

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