7
\$\begingroup\$

I wrote a little script that should load a GUI for deciding the bandpass filter. I'm a hobbyist programmer so I look here for comments that will help to improve my code abilities.

The applet should look like this:

enter image description here

# -*- coding: utf-8 -*-
"""
Created on Wed Jun 21 15:44:04 2017

@author: Mauro

The idea of the script is to create a small GUI managing the bandpass filter.
Since Tkinter can import only GIF image, an interface creates the images and
saves them in  a buffer folder, that are then read by the GUI class
"""

#==============================================================================
# Define Imports
#==============================================================================

# TkInter
from tkinter import (Tk, PhotoImage, Label, Frame, Canvas, Entry, Button, Menu,
                     filedialog, StringVar, IntVar, Checkbutton)

# sys imports
from os import mkdir 
from os.path import join, split, isdir, splitext
from shutil import rmtree
from copy import deepcopy

# matlibplot imports
from scipy.misc import imsave

# my imports
from MyImage_class import MyImage, Mask
from ImageFFT_class import ImgFFT

# todo:
    # save files button:      V
        # save composite      
    # add loading screen
    # support for RGB images

The class that calculates the Fourier transforms and the necessary images for the applet:

#==============================================================================
# Define Class managing image    
#==============================================================================

def get_pathname(path):
    ''' Little function to split the path in path, name, extension'''
    path, nameext = split(path)
    name, ext = splitext(nameext)
    return path, name, ext


class ImagePath:
    ''' Class that holds the filenames of the buffer folder and the images'''

    def __init__(self, name, image, bufpath):
        self.image = image
        self.name = name
        self.gifname = join(bufpath, self.name) + '.gif'

class ImageManager:
    ''' This class manages the images to be elaborated, it works in background 
    of the GUI to provide the transformations needed and the conversion to gif
    '''

    def __init__(self, imagepathname):

        # set the initial path and extract the approriate information
        self.initialpath = imagepathname

        path, name, ext = get_pathname(self.initialpath)
        self.mainpath = path
        self.name = name
        self.inimg_name = self.name + ext

        # create the directory for the elaboration
        self.bufpath = join(self.mainpath,self.name)
        if not isdir(self.bufpath):
            mkdir(self.bufpath)

        # open the source image
        self.inimage = ImagePath(self.name, MyImage(), self.bufpath)

        # declare the fourier transform
        self.ftimage = 0

        # TODO        
        # findppeak on the better cc
        # represent it with the tkinter canvas

    def init_inimage(self):
        # open the source image
        self.inimage = ImagePath(self.name, MyImage(), self.bufpath)
        self.inimage.image.read_from_file(self.initialpath)
        self.inimage.image.convert2grayscale()
        self.inimage.image.squareit()

        # resize the image and save it in gif format
        self.savegif(self.inimage, (500, 500))

    def calculate_bandpass(self, inradius, insmooth, outradius, outsmooth):
        ''' This method calculates the filter and saves the corresponding images
        the power spectrum (self.psimage) and the result of the filter
        (self.iftimage) in the temp folder
        '''

        #transfrom the image
        self.ftimage = ImgFFT(self.inimage.image)
        self.ftimage.ft()

        # create bandpass mask
        mask = Mask(self.inimage.image.data.shape)
        mask.bandpass(inradius, insmooth, outradius, outsmooth)
        self.ftimage.apply_mask(mask)

        # represent the masked ps
        self.ftimage.power_spectrum()
        self.psimage = ImagePath(self.name + "_ps", self.ftimage.ps, self.bufpath)
        self.savegif(self.psimage, (500, 500))        

        # calculate inverse transform
        self.ftimage.ift()
        self.iftimage = ImagePath(self.name + "ift", self.ftimage.imgifft, self.bufpath)
        self.savegif(self.iftimage, (500, 500))

    def savegif(self,imagepath, size):
        ''' Given a class imagepath and the size of the images, saves into the
        temp folder the associated image
        '''

        # calculate ft for resizing
        imft = ImgFFT(imagepath.image.data)
        imft.ft()
        im = imft.resize_image(size[0], size[1])

        # save resized image
        imsave(imagepath.gifname, im.data, format = "gif")

    def rm(self):
        ''' cleans up the buffer folder containing the gifs files'''
        if isdir(self.bufpath):
            rmtree(self.bufpath)

The class that actually manages the GUI. Since Tkinter is able to import only GIFs the applet looks for the GIF formatted images from the ManageImage class.

#==============================================================================
# Define GUI Object
#==============================================================================

class MyWidget(object):

    def __init__(self, root, mypathtoimage):
        # initialize the frame
        self.frame = Frame(root)
        self.frame.pack()    

        self.mypathtoimage = mypathtoimage

        # initialize helper class
        self.helper = ImageManager(self.mypathtoimage)

        # define the menu
        menubar = Menu(self.frame)

        menufile = Menu(menubar, tearoff = 0)
        menufile.add_command(label = "Open file...", command = self.openfile)
        menubar.add_cascade(label = "File", menu = menufile)

        root.config(menu = menubar)

        # TODO:
            # add save menu

        # add a label representing image data
        self.vstr_iminfo = StringVar()
        self.vstr_iminfo.set(" not init ")

        self.iminfolabel = Label(self.frame, textvariable = self.vstr_iminfo)
        self.iminfolabel.grid(row = 0, column = 0)

        # define the canvas display
        self.canvas = Canvas(self.frame, width=1500, height=500)
        self.canvas.grid(row = 1, column = 0)

        self.frame.image = {"inimage" : 0, "fft" : 0, "ift" : 0}
        self.canvasposition = {"inimage" : 0, "fft" : 500, "ift" : 1000}

        # create entries for bandpass
        l = Label(self.frame, text = "band pass values")
        l.grid(row = 2, column = 0)

        # create a new frame that contains 4 labels 4 entries and calculate button
        entryframe = Frame(self.frame)
        entryframe.grid(row = 3, column = 0) 

        # define the entry names
        myvariablesnames = ["inradius", "insmooth", "outradius", "outsmooth"]
        std_values = [str(e) for e in [5, 2, 100, 10]] # default bp values

        self.entries = {}
        for i, element in enumerate(myvariablesnames):
            Label(entryframe,text = element).grid(column = i * 2, row = 0)
            self.entries[element] = Entry(entryframe)
            self.entries[element].insert(0,  std_values[i])
            self.entries[element].grid(column = i * 2 + 1, row = 0)

        # define a button calculate
        b = Button(entryframe, text = "Calculate", command = self.calculate)
        b.grid(row = 0, column = 10)

        # build the save menu
        self.savedir = StringVar()
        self.savedir.set(" not init")

        # add save frame
        saveframe = Frame(self.frame)
        saveframe.grid(row = 4, column = 0)

        # define the save buttons. Transform it in a top level and call it once
        # is pressed on the menu
        self.myvariablesnames = ["wkdir", "saveps", "saveift", "savecomp"]
        self.defvalues = [self.savedir.get(),
                     self.helper.name + "_ps",
                     self.helper.name + "_ift",
                     self.helper.name + "_comp"]
        self.myvar_savefilenames = {}

        self.savefilesentry = {}
        for i, element in enumerate(self.myvariablesnames):
            Label(saveframe,text = element).grid(column = 0, row = i)

            self.myvar_savefilenames[element] = StringVar()
            self.myvar_savefilenames[element].set(self.defvalues[i])

            self.savefilesentry[element] = Entry(saveframe, textvariable = self.myvar_savefilenames[element])
            self.savefilesentry[element].grid(column = 1, row = i)

        # get directory to save pictures
        getdir = Button(saveframe, text = "get directory", command = self.getdirectory)
        getdir.grid(row = 0, column = 2)

        # create the check buttons to tell which images to save
        self.saveps = IntVar()
        self.saveift = IntVar()
        self.savecomp = IntVar()
        csaveps = Checkbutton(saveframe, text = "Save ps", variable = self.saveps,
                 onvalue = 1, offvalue = 0)
        csaveps.grid(column = 2, row = 1, sticky = "w")

        csaveps = Checkbutton(saveframe, text = "Save ift", variable = self.saveift,
                 onvalue = 1, offvalue = 0)
        csaveps.grid(column = 2, row = 2, sticky = "w")

        csaveps = Checkbutton(saveframe, text = "Save comp", variable = self.savecomp,
                 onvalue = 1, offvalue = 0)
        csaveps.grid(column = 2, row = 3, sticky = "w")

        #save button
        self.saveimgs_b = Button(saveframe, text = "save images",
                          state = 'disabled', command = self.saveimages)
        self.saveimgs_b.grid(column = 2, row = 4)

    def getdirectory(self):
        self.savedir.set(filedialog.askdirectory())

    def saveimages(self):
        ''' saves the displayed images from the original data'''
        wkdir = self.myvar_savefilenames["wkdir"].get()

        if self.saveps.get():
            print("saving ps image")
            filename = self.myvar_savefilenames["saveps"].get()
            print(wkdir)
            print(filename)
            self.helper.psimage.image.save(join(wkdir, filename + ".png"))

        if self.saveift.get():
            print("saving ift image")
            filename = self.myvar_savefilenames["saveift"].get()
            self.helper.iftimage.image.save(join(wkdir, filename + ".png"))    

        if self.savecomp.get():
            # not yet ready, adjust the images istograms to the 3sigma value
            print("saving composite image")
            filename = self.myvar_savefilenames["savecomp"].get()
            # put all 3 images together    
            newimage = deepcopy(self.helper.inimage.image)
            newimage.normalize()
            newimage.limit(1)

            psimage = self.helper.psimage.image
            psimage.normalize()
            psimage.limit(1)
            newimage.create_composite_right(psimage)

            iftimage = self.helper.iftimage.image
            iftimage.normalize()
            iftimage.limit(1)
            newimage.create_composite_right(iftimage)

            newimage.save(join(wkdir, filename + ".png"))

    def openfile(self):
        ''' The desired file gets initialized and updates the different fields'''
        # clean the folder
        self.helper.rm()

        # ask for filename
        self.mypathtoimage = filedialog.askopenfilename()

        # initializate the helper class
        self.helper = ImageManager(self.mypathtoimage)
        self.helper.init_inimage()

        # update label
        self.vstr_iminfo.set(self.buildinfostr())

        # update save file
        self.savedir.set(self.helper.mainpath)

        # update the default values
        self.defvalues = [self.savedir.get(),
                          self.helper.name + "_ps",
                          self.helper.name + "_ift",
                          self.helper.name + "_comp"]

        # update file names
        for i, element in  enumerate(self.myvariablesnames):
            self.myvar_savefilenames[element].set(self.defvalues[i])

        # show the first image
        self.show_image(self.helper.inimage, "inimage")


    def calculate(self):
        # get the bandpass valeus
        inradius = self.entries["inradius"].get()
        insmooth = self.entries["insmooth"].get()
        outradius = self.entries["outradius"].get()
        outsmooth = self.entries["outsmooth"].get()

        # apply the bandpass to the fourier transform
        self.helper.calculate_bandpass(int(inradius), int(insmooth), int(outradius), int(outsmooth))

        # represent the bandpass int
        self.show_image(self.helper.psimage, "fft")
        self.show_image(self.helper.iftimage, "ift")

        # unlock the button if a real image is imported
        self.saveimgs_b['state'] = 'normal'

    def show_image(self, image, name):
        ''' display the image in the canvas'''
        inimage = PhotoImage(file = image.gifname)
        self.frame.image[name] = inimage
        self.canvas.create_image(self.canvasposition[name],0 , image = inimage, anchor = "nw")

    def buildinfostr(self):
        ''' helper function to build the label infos'''
        name = self.helper.inimage.name
        inspect = self.helper.inimage.image.inspect()
        return name + " | " + inspect
#==============================================================================
# Test environment     
#==============================================================================

if __name__ == "__main__":

    # initializate Tk root
    root = Tk()    
    m = MyWidget(root, "../data/Lena.png")

    # start the loop
    root.mainloop()

    # clean up    
    m.helper.rm()

I have a GitHub repository. The GUI is actually part of a larger project and will be used to decide the bandpass filter to improve the cross correlation.

\$\endgroup\$
  • 1
    \$\begingroup\$ I have removed the off-topic portion of the question. If you want help with your installer problem, consult Stack Overflow, describing specifically what kind of error you are encountering. \$\endgroup\$ – 200_success Jun 25 '17 at 1:10
2
\$\begingroup\$

OO

  • Data-only classes like ImagePath are a code smell. Doesn't this information belong in MyImage?
  • You resize and save the image in the same method:

    # resize the image and save it in gif format
    self.savegif(self.inimage, (500, 500))
    

    To simplify and get rid of the comment you can pull out a method to resize the image, so the result looks something like this (also renamed things to make them easier to read):

    self.save(self.resize(self.image, 500, 500))
    
  • You have both myvariablesnames and self.myvariablesnames which is very confusing.
  • Use constants for values which should never change, such as self.myvariablesnames.

Oddities

  • At first glance this code makes no sense to me:

    # declare the fourier transform
    self.ftimage = 0
    

    Is it a numeric index into an array of Fourier [sic] transform functions? A measure of how much you're going to cut off the image in frequency space? Something else? Get rid of the comment and either make the attribute name obvious or get rid of it entirely.

  • This is even more confusing:

    self.ftimage = ImgFFT(self.inimage.image)
    

    Now you've used an attribute for an integer value and an object. I'm going to assume it never makes sense for the value to be an integer. If so, you can simply declare the value as None, the default "just creating a placeholder; please wait" code in Python.

  • The image saving UI is unusual. You might want to check with people at Graphic Design, but I think something like just an output directory and a save button (saving in all the formats) would be fine.

General

  • Creating and removing directories behind the scenes should only happen in a dedicated temporary workspace. Anything else is too risky.
  • This might be contentious, but import at the highest level possible (import x rather than from x import y) to avoid polluting the namespace. Aside from allowing you to implement functions with the same name as those in libraries without twiddling the imports (and thereby making them inconsistent), this makes it obvious which y you mean in your code. For example, join has several implementations, and the code has os.path.join(…) it's obvious which one is meant. Of course it's longer, but comprehension speed is more important than reading speed.
  • Run the code through pep8 (and possibly other linters), to help others and yourself (once you get used to idiomatic Python) to read the code faster.
  • Separate words in lowercase names by _ to make them readable.
  • Avoid prefixing things with My and suffixing classes with _class - Python has namespacing and a type system.
  • In general, spending more time on naming things can really help both comprehension and initial implementation. Think of it like this: MyImage or ImageManager could be and do anything related to images. They could even do the same thing. Bitmap and BandpassFilter restrict the class contents, avoiding scope creep by making it more obvious when you're overstepping their mandate. Once BandpassFilter does something other than applying a filter, such as saving, loading, removing or generating paths, you're more likely to notice and pause to think of either a better name or a better structure. (I'm not saying you should use these names specifically.)
\$\endgroup\$
  • \$\begingroup\$ First thank you for the thoughtful answer. The ImagePath class is a punch in the eye, but the MyImage class should be only for data handling (the only I/O functions attached are to import or export data), path and such should be handled somewhere else in the program, thus the stupid x.image, x.image_path link and x.name (is like an helper to remind me those things goes together) if you have a better implementation feel free to suggest. You got exactly the point self.ftimage should be initated to None How do I create a dedicated temporary space? I'm trying to learn PEP8 \$\endgroup\$ – Pella86 Jun 25 '17 at 10:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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