CODEWITHSUNDEEP
pythoneclipse
Big Al
Big Al

Reputation: 1030

Refactoring in Python (extract new method in Eclipse)

I am trying to refactor some Python 2.7 code into a new method in Eclipse. After I use Eclipse's Refactoring->Extract method on the block marked by comments below, my image no longer shows up in the GUI:

from Tkinter import Tk, Button, W, E, N, S, Label, Frame

from PIL import Image, ImageTk

def myCallback():
    pass

root = Tk()

# start refactor method here

root.geometry('400x400')
runButton = Button(root, text="Run",bg='green',
    command=myCallback)
runButton.grid(row=2, column=0,
        padx=10, pady=10)
quitButton = Button(root, text="Quit",
command=root.quit)
quitButton.grid(row=2, column=1,
        padx=10, pady=10)

frame1 = Frame(width=200, height=200)
frame1.grid(row=1, column=0, columnspan=1, rowspan=1,
        sticky=W+E+N+S, padx=10, pady=10)
image1 = Image.open("C:/Users/me/Pictures/house.jpg")
size = 64,64
image1.thumbnail(size, Image.ANTIALIAS)
photo1 = ImageTk.PhotoImage(image1) 
label1 = Label(image=photo1) 
label1.grid(row=0, column=10, columnspan=1, rowspan=1,
        sticky=N+E, padx=10, pady=10)

# end refactor method here

root.mainloop()

Could someone explain why the image disappears and suggest a solution such that I can refactor without losing the image?

After refactor:

from Tkinter import Tk, Button, W, E, N, S, Label, Frame

from PIL import Image, ImageTk

def extractedMethod(root):
    root.geometry('400x400')
    runButton = Button(root, text="Run", bg='green', command=myCallback)
    runButton.grid(row=2, column=0, padx=10, pady=10)
    quitButton = Button(root, text="Quit", 
    command=root.quit)
    quitButton.grid(row=2, column=1, padx=10, pady=10)
    frame1 = Frame(width=200, height=200)
    frame1.grid(row=1, column=0, columnspan=1, rowspan=1, sticky=W + E + N + S, padx=10, pady=10)
    image1 = Image.open("C:/Users/me/Pictures/house.jpg")
    size = 64, 64
    image1.thumbnail(size, Image.ANTIALIAS)
    photo1 = ImageTk.PhotoImage(image1)
    label1 = Label(image=photo1)
    label1.grid(row=0, column=10, columnspan=1, rowspan=1, sticky=N + E, padx=10, pady=10)

def myCallback():
    pass

root = Tk()
extractedMethod(root)

root.mainloop()

Thanks.

Upvotes: 1

Views: 871

Answers (2)

martineau
martineau

Reputation: 123541

One solution is to add one line to the beginning of theextractedMethod():

def extractedMethod(root):
    global photo1  # added
    ...

The problem is that otherwisephoto1is a local variable which is discarded as soon as the function returns, so references made to it within the function become invalid.

One might think that thelabel1 = Label(image=photo1)statement would have incremented its reference count and prevented this from happening, but it apparently doesn't work that way. I've seen this before and personally suspect it's due to a bug inTkinter...

Upvotes: 0

abarnert
abarnert

Reputation: 366103

The problem is that you're releasing the PhotoImage object too early.

As the docs explain:

You must keep a reference to the image object in your Python program, either by storing it in a global variable, or by attaching it to another object.

Note: When a PhotoImage object is garbage-collected by Python (e.g. when you return from a function which stored an image in a local variable), the image is cleared even if it’s being displayed by a Tkinter widget.

To avoid this, the program must keep an extra reference to the image object. A simple way to do this is to assign the image to a widget attribute, like this:

label = Label(image=photo)
label.image = photo # keep a reference!
label.pack()

You might think that passing it as the image argument to the Label constructor would keep it alive. But it doesn't; the Label doesn't actually keep a reference to the PhotoImage object itself, it's only the actual label construction in the Tcl code that runs after mainloop starts that does so. (It's similar to a weak reference, but not an intentional or explicit one, if that helps.)

So, the quick and dirty solution is to either add this to the top of extractedMethod:

global photo1

… or add this right after the label1 = Label(image=photo1) line:

label1.photo = photo1

A better way to avoid this is to use an OO design instead of a flat procedural design, so you have some reasonable object to own all your references.

Even the trivial version, as shown in these examples will work:

class Foo(object):
    def __init__(self, root):
        # put the code from extractedMethod here,
        # but prefix every local variable with self,
        # and do the same for myCallback.
    def myCallback(self):
        pass

But you're probably better off with a real OO design, where your objects actually represent something, as in the official examples:

class MyFrame(Frame):
    def __init__(self, master=None):
        Frame.__init__(self, master)
        self.pack()
        self.create_widgets()
    def myCallback(self):
        pass
    def create_widgets(self):
        # put the code from extractedMethod here,
        # again prefixing things with self

Upvotes: 2

Related Questions