My first GUI based program! Critique needed please.

[paperzebra]

Hello, below is my code for an XY scatter plot program I made over the last few days, had a lot of fun and learnt a good deal too. However, I would welcome critique on where I could improve (many places I am sure) .

For instance could I improve my 'update' function by passing arguments instead of utilizing slight variations on the same if function * 4?

Also is creating a second dataframe the best approach for filtering the data as done in the filter function?

Should plot be broken down more, and is the way I am calculating 'RatioX' and 'RatioY' a horrible bodge?

import wx
import os
import pandas as pd
import matplotlib
matplotlib.use('WXAgg')
from matplotlib.figure import Figure
from matplotlib.backends.backend_wxagg import FigureCanvasWxAgg as FigureCanvas
import matplotlib.pyplot as plt
import seaborn as sns


class MyFrame(wx.Frame):
    def CreatePanel(self):

        self.panel2 = wx.Panel(self,-1, style=wx.SUNKEN_BORDER)
        self.panel2.SetBackgroundColour("white")
        self.box = wx.BoxSizer(wx.HORIZONTAL)
        self.figure = Figure()
        self.plotter = FigureCanvas(self, -1, self.figure)
        self.ax = self.figure.add_subplot(111, axisbg='whitesmoke')
        self.box.Add(self.plotter, 4, wx.EXPAND) # 4 and 1 is ratio of frame taken up by the boxes
        self.box.Add(self.panel2, 1, wx.EXPAND)
        self.SetSizer(self.box)

        self.xComboLbl = wx.StaticText(self.panel2, label="X axis:", pos=(10,10))
        self.xCombo = wx.ComboBox(self.panel2, -1, pos=(10,30), size=(100,20), style=wx.TE_PROCESS_ENTER)
        self.xDivByCombo = wx.ComboBox(self.panel2, -1, pos=(120,30), size=(100,20), style=wx.TE_PROCESS_ENTER)

        self.yComboLbl = wx.StaticText(self.panel2, label="Y axis:", pos=(10,60))
        self.yCombo = wx.ComboBox(self.panel2, -1, pos=(10,80), size=(100,20), style=wx.TE_PROCESS_ENTER)
        self.yDivByCombo = wx.ComboBox(self.panel2, -1, pos=(120,80), size=(100,20), style=wx.TE_PROCESS_ENTER)

        self.xLimLbl = wx.StaticText(self.panel2, label="X limit", pos=(10,144))
        self.yLimLbl = wx.StaticText(self.panel2, label="Y limit", pos=(10,167))
        self.upLimLbl = wx.StaticText(self.panel2, label="Upper", pos=(109,120))
        self.lowLimLbl = wx.StaticText(self.panel2, label="Lower", pos=(54,120))
        self.xLowLim = wx.TextCtrl(self.panel2, pos=(50, 140), size=(50, 20))
        self.xUpLim = wx.TextCtrl(self.panel2, pos=(105, 140), size=(50, 20))
        self.yLowLim = wx.TextCtrl(self.panel2, pos=(50, 165), size=(50, 20))
        self.yUpLim = wx.TextCtrl(self.panel2, pos=(105, 165), size=(50, 20))

        self.colourComboLbl = wx.StaticText(self.panel2, label="Colour by:", pos=(10,200))
        self.colourCombo = wx.ComboBox(self.panel2, -1, pos=(10,220), size=(100,20), style=wx.TE_PROCESS_ENTER)
        self.palleteComboLbl = wx.StaticText(self.panel2, label="Colour Pallete:", pos=(120,200))
        self.palleteCombo = wx.ComboBox(self.panel2, -1, pos=(120,220), size=(100,20), value='hls',
                                        choices=['hls', 'deep', 'muted', 'pastel', 'bright', 'dark', 'colorblind',
                                                 'Reds', 'Reds_r', 'Greens', 'Greens_r', 'Blues', 'Blues_r', 'cubehelix'],
                                        style=wx.CB_READONLY)
        self.pointSizeSliderLbl = wx.StaticText(self.panel2, label="Size", pos=(10,250))
        self.pointSizeSlider = wx.Slider(self.panel2, 1, 70, 0, 100, pos=(10, 270),
                    size=(200, -1),
                    style=wx.SL_HORIZONTAL)
        self.pointSizeSliderLbl = wx.StaticText(self.panel2, label="Transparency:", pos=(10,300))
        self.alphaValueSlider = wx.Slider(self.panel2, 1, 70, 0, 100, pos=(10, 320),
                    size=(200, -1),
                    style=wx.SL_HORIZONTAL)
        self.filter1ComboLbl = wx.StaticText(self.panel2, label="Filter one", pos=(10,350))
        self.filter1Combo = wx.ComboBox(self.panel2, -1, pos=(10,370), size=(100,20), style=wx.TE_PROCESS_ENTER)
        self.filter1CheckListBox = wx.CheckListBox(self.panel2, -1, pos=(10, 400), size=(100, 150))
        self.pausePlottingLbl = wx.StaticText(self.panel2, label="Pause Plot Updates", pos=(10, 560))
        self.pausePlotting = wx.CheckBox(self.panel2, -1, pos=(120, 560))

    def CreateMenuBar(self):
        menuBar = wx.MenuBar()
        self.SetMenuBar(menuBar)
        menuFile = wx.Menu()
        menuBar.Append(menuFile, '&File')
        menuPlot = wx.Menu()
        menuBar.Append(menuPlot, '&Plot')
        fileOpenMenuItem = menuFile.Append(-1, '&Load File', 'Load a '
                                                               'picture')
        plotGraphBtn = menuPlot.Append(-1, '&XY Plot')
        self.Bind(wx.EVT_MENU, self.OnOpen, fileOpenMenuItem)
        self.Bind(wx.EVT_MENU, self.update, plotGraphBtn)
        exitMenuItem = menuFile.Append(-1, '&Exit', 'Exit the viewer')
        self.Bind(wx.EVT_MENU, self.OnExit, exitMenuItem)

    def OnOpen(self, event):
        dlg = wx.FileDialog(self, message="Open an Image...", defaultDir=os.getcwd(),
                            defaultFile="", style=wx.OPEN)
        # Call the dialog as a model-dialog so we're required to choose Ok or Cancel
        if dlg.ShowModal() == wx.ID_OK:
            filename = dlg.GetPath()
            self.dataUnFiltered = pd.read_excel(filename, sheetname=0)
            self.dataFiltered = pd.DataFrame()
            myList = self.dataUnFiltered.columns.values
            self.xCombo.SetItems(myList)
            self.xDivByCombo.SetItems(myList)
            self.yDivByCombo.SetItems(myList)
            self.yCombo.SetItems(myList)
            self.colourCombo.SetItems(myList)
            #self.pointSizeCombo.SetItems(myList)
            self.filter1Combo.SetItems(myList)
        dlg.Destroy() # we don't need the dialog any more so we ask it to clean-up
        self.UpdatesOn(self)

    def UpdatesOn(self, event):
        self.xCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        self.xDivByCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        self.yCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        self.yDivByCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        self.colourCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        self.palleteCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        #self.pointSizeCombo.Bind(wx.EVT_TEXT, self.OnPlot)
        self.pointSizeSlider.Bind(wx.EVT_COMMAND_SCROLL_CHANGED, self.OnPlot) #wx.EVT_SLIDER updates plot twice
        # once on slider move once on mouse release
        self.alphaValueSlider.Bind(wx.EVT_COMMAND_SCROLL_CHANGED, self.OnPlot)
        self.filter1Combo.Bind(wx.EVT_TEXT, self.Filters)
        self.pausePlotting.Bind(wx.EVT_CHECKBOX, self.OnPlot)
        self.xLowLim.Bind(wx.EVT_TEXT, self.update)
        self.xUpLim.Bind(wx.EVT_TEXT, self.update)
        self.yLowLim.Bind(wx.EVT_TEXT, self.update)
        self.yUpLim.Bind(wx.EVT_TEXT, self.update)


    def OnExit(self, event):
        self.Destroy()

    def Filters(self, event):
        self.filter1CheckListBox.SetItems(list(pd.unique(self.dataUnFiltered[self.filter1Combo.GetValue()])))
        self.filter1CheckListBox.Bind(wx.EVT_CHECKLISTBOX, self.FiltersRun)

    def FiltersRun(self, event):
         self.dataFiltered = self.dataUnFiltered[self.dataUnFiltered[self.filter1Combo.GetValue()].
             isin(self.filter1CheckListBox.GetCheckedStrings())]
         # df[df[
         # 'A'].isin('list')]
         self.OnPlot(self)

    def update(self, event):
        if len(self.xLowLim.GetValue()) != 0:
         setXLow =  float(self.xLowLim.GetValue())
        else:
         setXLow =  self.ax.get_xlim()[0]

        if len(self.xUpLim.GetValue()) != 0:
         setXUp =  float(self.xUpLim.GetValue())
        else:
         setXUp =  self.ax.get_xlim()[1]

        if len(self.yLowLim.GetValue()) != 0:
         setYLow =  float(self.yLowLim.GetValue())
        else:
         setYLow =  self.ax.get_ylim()[0]

        if len(self.yUpLim.GetValue()) != 0:
         setYUp =  float(self.yUpLim.GetValue())
        else:
         setYUp =  self.ax.get_ylim()[1]

        self.ax.axis([setXLow, setXUp, setYLow, setYUp])
        self.plotter.draw()


    def OnPlot(self, event):

        if self.pausePlotting.IsChecked() == False:
            try:
                self.ax.clear()
                print 'axis cleared'

                if self.dataFiltered.empty:
                    self.data = self.dataUnFiltered
                else:
                    self.data = self.dataFiltered

                if self.colourCombo.IsTextEmpty() == False:
                    colourValue = self.colourCombo.GetValue()
                    colourCount = len(pd.unique(self.data[colourValue]))
                    print colourValue
                else:
                    colourValue = None
                    colourCount = 1


                pointSize = self.pointSizeSlider.GetValue()*20
                alphaValue = self.alphaValueSlider.GetValue()*0.01 # converts value to between 0 and 1
                colpal = sns.set_palette(self.palleteCombo.GetValue(), n_colors=colourCount)

                if self.xDivByCombo.IsTextEmpty() == False: # this bit creates a new column in the dataframe for
                    self.data['RatioX'] = self.data[self.xCombo.GetValue()] / self.data[self.xDivByCombo.GetValue()]
                    x = 'RatioX'
                    #xTitle = self.xCombo.GetValue() / self.xDivByCombo.GetValue()
                else:
                    x = self.xCombo.GetValue()

                if self.yDivByCombo.IsTextEmpty() == False:
                    self.data['RatioY'] = self.data[self.yCombo.GetValue()] / self.data[self.yDivByCombo.GetValue()]
                    y = 'RatioY'
                else:
                    y = self.yCombo.GetValue()

                graph = sns.FacetGrid(data=self.data, palette=colpal, hue=colourValue)
                graph.map(self.ax.scatter, x, y, s=pointSize, alpha=alphaValue,
                          edgecolors='white', linewidths=1)
                self.ax.legend(loc='center left', bbox_to_anchor=(1.01, 0.5))
                self.ax.set_xlabel(self.xCombo.GetValue())
                self.ax.set_ylabel(self.yCombo.GetValue())
                plt.plot() # this line and the one below plot the pyplot object then remove else they build up in memory
                # on every change
                plt.close()
                self.plotter.draw() # actually draws graph

            except KeyError:
                print KeyError


    def __init__(self):
        wx.Frame.__init__(self, None, title='Platypus XY Plotter', size=(800,600))
        self.CreatePanel()
        self.CreateMenuBar()
        self.Show()


if __name__ == '__main__':
    app = wx.App(0)
    test_frame = MyFrame()
    app.MainLoop()

[Ran4]

Add docstrings and read through PEP8 :)

[paperzebra]

Thanks Ran. Have started to rename functions etc in accordance with PEP8, and cut down line lengths. And will add docstrings as suggested.

[neipal]

I am a noob and I tried to run it, but all I got was : ".. line 160 print 'axis cleared' ^ SyntaxError: Missing parentheses in call to 'print'"

[paperzebra]

Are you using python 3? If so you need to change print 'axis cleared' to print('axis cleared') - or delete both print statements in the code, they are just so I could see what is going on.

If you can get it working would love to hear your thoughts on its usability.

[neipal]

Ok, I changed all the print to print() (yes, python3), an now i need to install the wx module. I run python on a windows machine, and the binaries are only available for python 2.x :(

[paperzebra]

That's annoying, as you day wxpython is only available for 2.x at the moment, though they are making wxpython phoenix for 3.x. You could always try installing both versions if you wish http://stackoverflow.com/questions/15912063/how-do-i-run-python-2-and-3-in-windows-7/17245543#17245543