[Req4uim]

I'm relatively new to python. The code below is a snipetfrom a project I made using turtle. It is functional but the thing is I want to improve upon it.

The main problem I think is that it is rather clunky as I constantly repeat virtually the same staements over and over. If anyone can point me in the right direction as to simplifying and improvintg it would be greatly appreaciated.

def colour_range():
    def dblue_colour():
        pencolor("dark blue")
        fillcolor("dark blue")
    def blue_colour():
        pencolor("blue")
        fillcolor("blue")
    def dred_colour():
        pencolor("dark red")
        fillcolor("dark red")
    def red_colour():
        pencolor("red")
        fillcolor("red")     
    def dgreen_colour():
        pencolor("dark green")
        fillcolor("dark green")
    def green_colour():
        pencolor("green")
        fillcolor("green")
    def black_colour():
        pencolor("black")
        fillcolor("black")
    def white_colour():
        pencolor("white")
        fillcolor("white")
    def purple_colour():
        pencolor("purple")
        fillcolor("purple")
  
    onkey(dblue_colour, "1")
    onkey(blue_colour, "2")
    onkey(dred_colour, "3")
    onkey(red_colour, "4")
    onkey(dgreen_colour, "5")
    onkey(green_colour, "6")
    onkey(black_colour, "7")
    onkey(white_colour, "8")
    onkey(purple_colour, "9")

[jon.kiparsky]

First of all, your instincts are correct. "Don't repeat yourself" is a good rule, so pat yourself on the back for seeing what needed to be done.

One way to clean this up - and there are probably better ones, I'm new to python myself - would be to make a dictionary of colour names and use the magic of lambda and zip.

If you're not familiar with lambda functions, they're a concept taken from lisp: a lambda is a function* which returns a function. Start up a python instance and try this:

def return_x(x):
  return x

A simple and totally useless function, but it serves the purpose for an example. Okay, now do this:

f = lambda: return_x(4)

Now f is a function, so you can call it:

f()

As you can see, I've just turned a function of one variable into a function of no variables - which is what you needed for your onkey function.

So if you had a function which took a colour name as an argument, and a list of colours, you could generate a list of functions just like your ones (although they wouldn't be named).

Something like:

colour_fns = []
for colour in colours:
  colour_fns.append(lambda : set_colour(colour))

But you want to associate each of those functions with a number. In other language you'd use a for loop to make a counter, but here you'd probably want to use a zip. Zip() takes lists and turns them into lists of tuples. Very useful for iterating two lists at once, since you can implicitly disaggregate the tuples. Here's a toy example.

list1= [1,2,3,4]
list2= ['a','b','c','d']

for num,ltr in zip(list1, list2):
  print ltr, num

If you just do

zip(list1, list2)

you'll see the internal structure of a zipped list.


So in this case, I'd probably do something like this:

for colour, key in zip(colours, range(1,9)):
  onkey(lambda: set_colour(colour), key)

I've intentionally left some blanks here for you to fill in. I may have also unintentionally introduced some errors, since I'm doing this all in the editor. I think you'll be able to figure it out from here, so give it a good shot.


Notice that the syntax of lambda is a little weird. "A function that returns a function" is the lisp definition. If it's not technically strictly properly a function in python, I apologize for any confusion this causes.
Lambdas are very useful, and can be used in all sorts of cool ways. For example, they're handy for map, filter and reduce - but for now, I'll say no more about that, there's enough here for now.

[Req4uim]

Thank you so much this is exactly what I was looking for.

I'm glad you left parts out I hate getting handed the answers as I learn nothing.

[baavgai]

Python programmers love lists. So much so, we might get carried away with them. While lambda is a nice short hand, it's never required. To pass a function reference with values pre loaded, you need only a little closure magic.

With this in mind...

def colour_range():
	def setup_colour(key, pen, fill=None):
		def onkey_call():
			pencolor(pen)
			fillcolor(fill)
		if not fill:
			fill = pen
		onkey(onkey_call, key)
  
	setup_color("1", "dark blue")
	setup_color("2", "blue")
	# ...

[jon.kiparsky]

Nice.
I'd still consider using a list to call setup_color, partly because it's a little neater to my eye and partly because it sets you up to do something like this:

def bind_colour_keys(colour_list):
  for colour, key in zip (colour_list, range(1, len(colour_list)+1)):
     setup_colour(str(key), colour)


bind_colour_keys(colours, range(1,10))

Now it's a little more flexible - it's easier to use an arbitrary list of colours.

That's a relatively trivial point, however - this is a nice clean solution for the problem.

[baavgai]

I considered, and started typing, a list. Then I thought, what if my keys aren't sequential? Then I'd have to type the key-color tuples no matter what, so looping seemed extra.

To be honest, the zip thing wouldn't have occurred to me. I would have gone with:

def bind_colour_keys(colour_list):
	for i in range(len(colour_list)):
		setup_colour(str(i+1), colour_list[i])

[jon.kiparsky]

I considered the question of different key ranges. For simplicity, I left it out, but what I started with was something like

def bind_colour_keys(colour_list, key_list):
  for colour, key in zip (colour_list, key_list):
     setup_colour(key, colour)


bind_colour_keys(colours, keys)

Now you have to either manually or programmatically generate ["1", "2", "3"...], so that's a little bit of a loss, but you can also use ["q", "w", "e", ...] or ["!", "@", "#", ...] or whatever.

As for the zips, I've really been taken by them as a Really Nice Feature of python. I really like them, semantically, for iterating related lists, and since ranges are sequences, I use them there as well. Probably idiosyncratic and not idiomatic, but that is in fact how I roll.

[atraub]

This is where one of my favorite features of Python comes in, enumerate

def bind_colour_keys(colour_list):
	for index, color in enumerate(colour_list):
		setup_colour(str(index+1), color)

This, of course, assumes that the keys are logically related to the order of the list, but it does have a certain Pythonic elegance to it.

This post has been edited by atraub: 28 August 2012 - 08:01 AM

[Req4uim]

Well I got the colours working and it looks a lot better so I'm happy about that. I couldn't def bind_colours_key as it kept resulting in NameError: global name 'setup_colour' is not defined.

def colour_range():    
    def setup_colour(key, pen, fill=None):
        def onkey_call():
            pencolor(pen)
            fillcolor(fill)
        if not fill:
            fill = pen
        onkey(onkey_call,key)
    for i in range(len(colour_list)):
        setup_colour(str(i+1), colour_list[i])

However I've run into a few snags here and there...and I've just been running in circles confusing myself.

# These are just lists with the keys I'm trying to use and the default cursor shapes in turtle.
default_shapes = ["circle", "arrow", "square", "turtle", "classic"]
stamp_keys = ["a", "s", "d", "f", "g"]

# This one attempts to use Lambda but I keep confusing myself.
def alternating_stamps():
    def stamp_settings(key, shape_type):
        def onkey_call():
            shape(default_shapes)
    for key,shape_type in zip(stamp_keys, default_shapes):
        onkey(lambda: key, shape_type)

#Resultints in this error:
TclError: bad event type or keysym "arrow"


# When just using zip 
def alternating_stamps():
    def stamp_settings(key, shape_type):
        def onkey_call():
            shape(default_shapes)
        onkey(onkey_call, key)
    for key,shape_type in zip(stamp_keys, default_shapes):
        stamp_settings(str(key), shape_type)

Exception in Tkinter callback
TurtleGraphicsError: There is no shape named ['circle', 'arrow', 'square', 'turtle', 'classic']

[baavgai]

Wait, the code I offered didn't work?!?

def pencolor(pen):
	print "pencolor", pen

def fillcolor(fill):
	print "fillcolor", fill

def onkey(f, key):
	print "onkey", key
	f()

def colour_range():    
	def setup_colour(key, pen, fill=None):
		def onkey_call():
			pencolor(pen)
			fillcolor(fill)
		if not fill:
			fill = pen
		onkey(onkey_call,key)
	for i, colour in enumerate(['red','blue']):
		setup_colour(str(i+1), colour)

Tested, works fine. Check your indents. You want all spaces or all tabs; never mix.

Forget the looping. Can you get it to work with one case? Also, with your given code, I'd probably go back to my first example.

Avoid globals where possible. You might have something lurking around in the global space biting you.

( I liked atraub's enumerate. Used it in above test instead of original. )

[Req4uim]

Oh no it worked I just thought that for this code below

def colour_range():    
    def setup_colour(key, pen, fill=None):
        def onkey_call():
            pencolor(pen)
            fillcolor(fill)
        if not fill:
            fill = pen
        onkey(onkey_call,key)
    def bind_keys_colour{colour_list}:
    for i in range(len(colour_list)):
        setup_colour(str(i+1), colour_list[i])

It works. Argg I tried to tab in to edit and accidentally posted before finished writing. I thought you first meant to define another function sort of like below but I kept receiving errors. So I just put in the for statements and it works fine so no problem.

def colour_range():   
    def setup_colour(key, pen, fill=None):
        def onkey_call():
            pencolor(pen)
            fillcolor(fill)
        if not fill:
            fill = pen

    def bind_keys_colour{colour_list}:
       for i in range(len(colour_list)):
           setup_colour(str(i+1), colour_list[i])