Image editing in the frequency space

Question

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.

• 200_success: 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.

Answer 1

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.)

• Pella86: 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