Plotting Whether Measured Locations for Components are Within Spec

Question

I've made this GUI program for my workplace that checks whether measured coordinates on machined components are within spec.

It takes in specified coordinates, tolerance and measured coordinates and uses Pythagoras to check whether the tolerance has been exceeded.

It then plots the measured point relative to the 'true point' on a Tkinter canvas which can be scaled up and down with the slider widget (points are scaled proportionally - this was the challenging bit for me). The tolerance value determines the radius of the circle shown below.

I've ran the program through Pylint and am now getting good results (9.72). I'm now hoping for a bit of feedback on anything I could have done better. I was also trying to work to OOP ideas. Any comments about these or other areas would be much appreciated.

**"""Validates points against engineering drawing according to specified tolerance"""**

from tkinter import   Tk, Label, Entry, GROOVE, Button, IntVar, Canvas, Scale, Checkbutton

class TruePosition:
    """Populates Tk window object with widgets"""
    def __init__(self, master):
        self.master = master
        master.title('True Position Validator')

#Labels

        self.truelabel = Label(master, text='True Point Location Details:', font=('Technic', 14))
        self.hlabel = Label(master, text='Point Loc:', font=('Technic', 12))
        self.commalabel = Label(master, text=', ')
        self.tollabel = Label(master, text='Tolerance:', font=('Technic', 12))
        self.measlabel = Label(master, text='Measured Point Location Details', font=('Technic', 14))
        self.mlab = Label(master, text='Point Loc:', font=('Technic', 12))
        self.commalabel2 = Label(master, text=', ')
        self.labelout = Label(master, fg='black', font=('Technic', 12))

# Entries

        self.h_cor = Entry(master, relief=GROOVE)
        self.k_cor = Entry(master, relief=GROOVE)
        self.tol_cor = Entry(master, relief=GROOVE)
        self.x_cor = Entry(master, relief=GROOVE)
        self.y_cor = Entry(master, relief=GROOVE)
        self.entries = [self.h_cor, self.k_cor, self.tol_cor, self.x_cor, self.y_cor]

# Running Variables

        self.cross = False
        self.checkvar = IntVar()
        self.stored_points = {}
        self.count = 0
        self.line_length = 10
        self.h_val = None
        self.k_val = None
        self.tol_val = None
        self.x_val = None
        self.y_val = None

        self.real = None
        self.clearance = None
        self.radius = None

        self.linea_x1 = None
        self.linea_x2 = None
        self.linea_y = None
        self.lineb_y1 = None
        self.lineb_y2 = None
        self.lineb_x = None

        self.linea = None
        self.lineb = None

# Interactive / Function Call Elements

        self.retain = Checkbutton(master, text='Retain plots', variable=self.checkvar,
                                  command=lambda: self.box_check())
        self.bsubmit = Button(master, text='Submit', font=('Technic', 10),
                              command=lambda: self.submit_action())
        self.master.bind("<Return>", lambda t: self.submit_action())
        self.bclear = Button(master, text='Clear', font=('Technic', 10),
                             command=lambda: self.clearcrosses())
        self.slider = Scale(master, orient='horizontal', from_=0, to=100,
                            command=lambda factor: self.scalar(factor=float(factor)))
        self.slider.set(100)

# Base Graphics

        # Canvas

        self.display_canvas = Canvas(master, bg='white', border=2, highlightthickness=1,
                                     highlightbackground='black')
        self.truecircle = self.display_canvas.create_oval(0, 0, 0, 0, outline='black', dash=(4, 5))

        # Circle Centre Point

        pointa_x1 = (self.display_canvas.winfo_reqwidth()/2)-self.line_length/2
        pointa_x2 = (self.display_canvas.winfo_reqwidth()/2)+self.line_length/2
        pointa_y = self.display_canvas.winfo_reqheight()/2

        pointb_y1 = (self.display_canvas.winfo_reqheight()/2)-(self.line_length/2)
        pointb_y2 = (self.display_canvas.winfo_reqheight()/2)+(self.line_length/2)
        pointb_x = self.display_canvas.winfo_reqwidth()/2

        points = (pointa_x1, pointa_x2, pointa_y, pointb_y1, pointb_y2, pointb_x)

        self.display_canvas.create_line(points[0], points[2], points[1], points[2])
        self.display_canvas.create_line(points[5], points[3], points[5], points[4])

# Geometry Management

        self.truelabel.grid(row=0, column=0, columnspan=4, pady=8, sticky='w')
        self.hlabel.grid(row=1, column=0, sticky='w')
        self.commalabel.grid(row=1, column=2)
        self.tollabel.grid(row=2, column=0, sticky='w')
        self.measlabel.grid(row=4, sticky='w', pady=8)
        self.mlab.grid(row=5, column=0, sticky='w')
        self.commalabel2.grid(row=5, column=2)
        self.labelout.grid(row=7, sticky='w', columnspan=4)

        self.h_cor.grid(row=1, column=1)
        self.k_cor.grid(row=1, column=3)
        self.tol_cor.grid(row=2, column=1)
        self.x_cor.grid(row=5, column=1)
        self.y_cor.grid(row=5, column=3)

        self.retain.grid(row=8, column=3, sticky='e')
        self.bsubmit.grid(row=6, column=1, sticky='w')
        self.slider.grid(row=9, columnspan=4)
        self.bclear.grid(row=8, column=3, sticky='ne', padx=40)
        self.display_canvas.grid(row=8, columnspan=4, sticky='s')

        self.canvas_width = self.display_canvas.winfo_reqwidth()
        self.canvas_height = self.display_canvas.winfo_reqheight()

# Functions
    def box_check(self):
        """Acknowledges box check & stores existing point if switching from non retention mode"""
        if self.checkvar.get() == 1 and self.cross:
            self.store_points()
        elif self.checkvar.get() == 0:
            self.clearcrosses()

        print('ran box_check\n')

    def submit_action(self):
        """Accepts user input and runs subsequent functions"""
        try:
            self.calctrue()
            self.showtrue()
            self.meas_point(self.x_val, self.y_val)
            if not self.cross or self.checkvar.get() == 1:
                self.create_points()
                if self.checkvar.get() == 1:
                    self.store_points()
                else:
                    pass
            elif self.cross:
                self.move_points(self.linea, self.lineb)
        except ValueError: #  Accounts for accidental pressing of submit before all data is entered
            self.labelout.config(fg='black')
            self.labelout['text'] = 'ValueError : Make sure all fields have been completed.'

        print('ran submit_action()\n')

    def scalar(self, factor):
        """Runs logic that changes circle size in response to
        slider and remeasures & shifts existing points"""
        self.getplot_circ(factor)
        if self.checkvar.get() == 0 and self.cross:
            self.meas_point(self.x_val, self.y_val)
            self.move_points(self.linea, self.lineb)
        elif self.checkvar.get() == 1 and self.cross:
            for point in self.stored_points:
                self.meas_point(*self.stored_points[point][1])
                self.move_points(*self.stored_points[point][0])
        elif not self.cross:
            pass

        print('ran scalar\n')

    def getplot_circ(self, factor):
        """Changes circle size"""
        factor = 192 - (factor*1.27)

        self.radius = (self.canvas_width/2)- factor

        c1_x = (self.canvas_width/2) - self.radius
        c1_y = (self.canvas_height/2) - self.radius
        c2_x = (self.canvas_width/2) + self.radius
        c2_y = (self.canvas_height/2) + self.radius

        self.display_canvas.coords(self.truecircle, c1_x, c1_y, c2_x, c2_y)

        print('ran getplot_circ()')

    def calctrue(self):
        """Determines if point is within spec"""
        self.h_val = float(self.h_cor.get())
        self.k_val = float(self.k_cor.get())
        self.tol_val = float(self.tol_cor.get())
        self.x_val = float(self.x_cor.get())
        self.y_val = float(self.y_cor.get())

        self.real = (((self.x_val-self.h_val)**2)+((self.y_val-self.k_val)**2))**0.5
        self.clearance = self.tol_val - self.real

        print('ran calctrue()')

    def showtrue(self):
        """Provides non-graphical feedback about results of calctrue"""
        if self.tol_val >= self.real:
            self.labelout.config(fg='green')
            self.labelout['text'] = f'Position is within spec by {self.clearance}'
            self.display_canvas.itemconfig(self.truecircle, outline='green')
        elif self.tol_val < self.real:
            self.labelout.config(fg='red')
            self.labelout['text'] = f'Position is outwith spec by {abs(self.clearance)}'
            self.display_canvas.itemconfig(self.truecircle, outline='red')

        print('ran showtrue()')

    def meas_point(self, loc_x, loc_y):
        """defines new point in terms of current circle & canvas geometry"""
        conversionfactor = self.radius/self.tol_val
        pix_x = (conversionfactor * (loc_x-self.h_val)) + self.canvas_width/2
        pix_y = (-conversionfactor * (loc_y-self.k_val)) + self.canvas_height/2

        self.linea_x1 = (pix_x - (self.line_length/2))
        self.linea_x2 = (pix_x + (self.line_length/2))
        self.linea_y = (pix_y)

        self.lineb_y1 = (pix_y - (self.line_length/2))
        self.lineb_y2 = (pix_y + (self.line_length/2))
        self.lineb_x = (pix_x)

        print('ran meas_point()')

    def move_points(self, linea, lineb):
        """Shifts cross point to measured locations"""
        self.display_canvas.coords(linea, self.linea_x1, self.linea_y,
                                   self.linea_x2, self.linea_y)
        self.display_canvas.coords(lineb, self.lineb_x, self.lineb_y1,
                                   self.lineb_x, self.lineb_y2)

        print('ran move_points()')

    def create_points(self):
        """Plots new cross point"""
        self.linea = self.display_canvas.create_line(self.linea_x1, self.linea_y,
                                                     self.linea_x2, self.linea_y, tag='cross')
        self.lineb = self.display_canvas.create_line(self.lineb_x, self.lineb_y1,
                                                     self.lineb_x, self.lineb_y2, tag='cross')

        if not self.cross:
            self.cross = True

        print('ran create_points()')

    def store_points(self):
        """stores points for rescaling if slider is used"""
        print(self.count)
        self.stored_points[self.count] = [[self.linea, self.lineb], [self.x_val, self.y_val]]
        self.count += 1

        print('ran store_points()')

    def clearcrosses(self):
        """deletes all user input"""
        for item in self.display_canvas.find_withtag('cross'):
            self.display_canvas.delete(item)
        self.labelout['text'] = ''
        self.display_canvas.itemconfig(self.truecircle, outline='black')
        self.cross = False
        self.stored_points = {}

        for instance in self.entries:
            instance.delete(0, 'end')

        print('ran clearcrosses()\n')

ROOT = Tk()
MYAPP = TruePosition(ROOT)

ROOT.mainloop()

Answer 1

There is a lot to unpack here, so this review will be in multiple edits.

GUI Creation

You are creating and storing some widgets unnecessarily in member variables. For instance, self.truelabel is only used in the TruePosition constructor, it could be a local variable instead of a member variable. The same is true for almost all of the other labels.

Your GUI creating is a very manually intensive process. You create each item, and later position it to absolute coordinates. Let's let Python help us out here, and figure out the coordinates itself.

class TruePosition:

    """Populates Tk window object with widgets"""
    def __init__(self, master):
        self.master = master
        master.title('True Position Validator')

        self.heading('True Point Location Details:')
        self.heading('Measured Point Location Details')

    def heading(self, title):
        _, rows = self.master.grid_size()
        label = Label(self.master, text=title, font=('Technic', 14))
        label.grid(row=rows, columnspan=4, sticky='w')


ROOT = Tk()
MYAPP = TruePosition(ROOT)

ROOT.mainloop()

This short little program just creates the two heading lines in your GUI. Note that the first heading is on row=0 and the second on row=1, but we never specified that. We just created self.heading(...) lines, and the heading(self, title) method determine what the next row number was, created the label as a local variable, and positioned it all on its own.

Building on that, lets add the first 5 entry fields. First, add this additional import:

from tkinter import DoubleVar

TkInter has some special "Vars" that can be used to pass data into and out of control widgets. A DoubleVar does this for floating point values, including the conversion to and from a string.

        self.h_cor = DoubleVar()
        self.k_cor = DoubleVar()
        self.tol_cor = DoubleVar()
        self.x_cor = DoubleVar()
        self.y_cor = DoubleVar()

        self.heading('True Point Location Details:')
        self.row('Point Loc:', self.h_cor, ',', self.k_cor)
        self.row('Tolerance:', self.tol_cor)
        self.heading('Measured Point Location Details')
        self.row('Point Loc:', self.x_cor, ',', self.y_cor)

Now, we've created a DoubleVar for each of the 5 entry fields, and we're passing these variables, along with some text to row() functions. Let's define those:

    def row(self, *fields):
        _, rows = self.master.grid_size()
        for col, field in enumerate(fields):
            if isinstance(field, str):
                label = Label(self.master, text=field, font=('Technic', 12))
                label.grid(row=rows, column=col, sticky='w')
            elif isinstance(field, DoubleVar):
                entry = Entry(self.master, textvar=field, relief=GROOVE)
                entry.grid(row=rows, column=col)

Again, we're letting Python keep track of which row we're on. For each of the fields we pass in (the arguments to the row(...) call), we check if we were given a string or not. If a string was given, we create a label and position it. Otherwise, we create an entry using that field as the textvar, and position it.

If you run this short little program, you'd see we've created the first 5 lines of your GUI. Notice how easy it would be to add new items or change the layout; simply call the heading() or row() functions in the correct order.

To use the DoubleVar, simply .get() the values. Ie)

        self.h_val = float(self.h_cor.get())

becomes

        self.h_val = self.h_cor.get()

To add Button and Slider objects to our grid, it is useful to pass in other Widget objects (from tkinter import Widget) to our row() method. We can broaden DoubleVar to Variable to allow an Entry widget to be created if passed a StringVar or an IntVar as well.

    def row(self, *fields):
        _, rows = self.master.grid_size()
        for col, field in enumerate(fields):
            if isinstance(field, str):
                label = Label(self.master, text=field, font=('Technic', 12))
                label.grid(row=rows, column=col, sticky='w')
            elif isinstance(field, Variable):
                entry = Entry(self.master, textvar=field, relief=GROOVE)
                entry.grid(row=rows, column=col)
            elif isinstance(field, Widget):
                field.grid(rows=row, column=col)

Sometimes, we will want to get the widgets that are created. For instance, we would want the labelout widget that gets created, so we can change the colour of the label programmatically. So, let's return a list of all the widgets that are created:

    def row(self, *fields):
        _, rows = self.master.grid_size()
        widgets = []

        for col, field in enumerate(fields):
            widget = None
            if isinstance(field, str):
                widget = Label(self.master, text=field, font=('Technic', 12))
            elif isinstance(field, DoubleVar):
                widget = Entry(self.master, textvar=field, relief=GROOVE)
            elif isinstance(field, Widget):
                widget = field

            if widget:
                widget.grid(row=rows, column=col, sticky='w')
                widgets.append(widget)

        return widgets

Now we can create more of the GUI with:

        self.heading('True Point Location Details:')
        self.row('Point Loc:', self.h_cor, ',', self.k_cor)
        self.row('Tolerance:', self.tol_cor)

        self.heading('Measured Point Location Details')
        self.row('Point Loc:', self.x_cor, ',', self.y_cor)
        btn = Button(master, text="Submit", font=('Technic', 10),
                     command=self.submit_action)
        self.row(None, None, btn)

        self.labelout = self.row("")[0]

        self.display_canvas = Canvas(master, bg='white', border=2, highlightthickness=1,
                                     highlightbackground='black')
        self.display_canvas.grid(columnspan=4, sticky='s')

Math

You're doing some calculations the hard way.

        self.real = (((self.x_val-self.h_val)**2)+((self.y_val-self.k_val)**2))**0.5

could be written much simpler:

       self.real = math.hypot(self.x_val - self.h_val, self.y_val - self.k_val)

Don't use a Dictionary as a List

This code is a poor design choice:

    self.stored_points = {}
    self.count = 0

First, count is keeping track of the number of elements in store_points. Python contains know how many items are in the container; just ask for the len(container), and you'll get the equivalent of count.

Secondly, the data is being stored using the current count value as a key, which means you store the data under key 0, then 1, then 2, and so on. This is the hard-way of looking up items in a list! You should just use a list!

    self.stored_points = []

And append new items to the list:

    self.stored_points.append(((self.linea, self.lineb), (self.x_val, self.y_val)))

And iterate over the list:

        for pnt0, pnt1 in self.stored_points:
            self.meas_point(*pnt1)
            self.move_points(*pnt0)

Crosses

You are doing a lot of work to create crosses. A lot of unnecessary information is being stored in members (linea, lineb, linea_x1, linea_x2, ... lineb_x)

Instead, it would make sense to have a function which creates crosses on the canvas, at an (x,y) location.

    def cross(self, x, y, tag):
        scale = self.radius / self.tol_cor.get()
        half_len = self.line_length / 2
        center_x = self.canvas_width / 2
        center_y = self.canvas_height / 2
        x = center_x + scale * (x - self.h_cor.get())
        y = center_y - scale * (y - self.k_cor.get())

        self.display_canvas.create_line(x, y - half_len, x, y + half_len, tag=tag)
        self.display_canvas.create_line(x - half_len, y, x + half_len, y, tag=tag)

Calling self.cross(self.h_cor.get(), self.k_cor.get(), None) during the constructor would create the centre cross with no tag. Calling self.cross(self.x_cor.get(), self.y_cor.get(), "cross") would create a measurement point cross with the tag "cross".