views:

81

answers:

3

I am unable to get this code to sort a list of objects using either .sort() or sorted(). What am I missing here?

P.S. My solution.distance() method could use some cosmetic surgery too if anyone has any suggestions.

Thanks!

import random
import math

POPULATION_SIZE = 100

data = [[1, 565.0, 575.0],
        [2, 25.0, 185.0],
        [3, 345.0, 750.0],
        [4, 945.0, 685.0],
        [5, 845.0, 655.0],
        [6, 880.0, 660.0],
        [7, 25.0, 230.0],
        [8, 525.0, 1000.0],
        [9, 580.0, 1175.0],
        [10, 650.0, 1130.0]
        ]

class Solution():

  def __init__(self):
    self.dna = []
    self.randomize()

  def randomize(self):
    temp = data[:]
    while len(temp) > 0:
      self.dna.append( temp.pop( random.randint( 0,len(temp)-1 ) ) ) 

  def distance(self): 
    total = 0 
    #There has to be a better way to access two adjacent elements.
    for i, points in enumerate(self.dna):
      if i < (len(self.dna)-1): 
        total += math.sqrt( (points[1]-self.dna[i+1][1])**2 + (points[2]-self.dna[i+1][2])**2 )
      else:
        total += math.sqrt( (points[1]-self.dna[0][1])**2 + (points[2]-self.dna[0][2])**2 )
    return int(total)


class Population():

  def __init__(self):
    self.solutions = []
    self.generation = 0

    #Populate with solutions
    self.solutions = [Solution() for i in range(POPULATION_SIZE)]


  def __str__(self):

    result = ''

    #This is the part that is not returning sorted results.  I tried sorted() too.
    self.solutions.sort(key=lambda solution: solution.distance, reverse=True)


    for solution in self.solutions:
      result += 'ID: %s - Distance: %s\n' % ( id(solution),  solution.distance() )

    return result


if __name__ == '__main__':

  p = Population()
  print p
+3  A: 

Change

key=lambda solution: solution.distance

to

key=lambda solution: solution.distance()

(The parentheses are needed to call the function.)

Alternatively, you could make the distance method a property:

  @property
  def distance(self): 
      ....

In this case, change all occurances of solution.distance() to solution.distance. I think this alternate solution is a little bit nicer, since it removes two characters of clutter (the parens) every time you wish to talk about the distance.

PS. key=lambda solution: solution.distance was returning the bound method solution.distance for each solution in self.solutions. Since the same object was being returned as the key for each solution, no desired ordering occurred.

unutbu
The @property decorator worked for me. Thanks for the help!
Alex
+1  A: 

Here's an attempt at cleaning up your class, using functional programming techniques:

import random

class Solution():

  def __init__(self):
    self.dna = []
    self.randomize()

  def randomize(self):
    self.dna = data
    random.shuffle(self.dna)

  def distance(self):
    # Return the distance between two points.
    def point_distance((p1, p2)):
      return math.sqrt((p1[1]-p2[1])**2) + (p1[2]-p2[2])**2)
    # sums the distances between consecutive points.
    # zip pairs consecutive points together, wrapping around at end.
    return int(sum(map(point_distance, zip(self.dna, self.dna[1:]+self.dna[0:1])))

This is untested, but should be close to working. Also, a suggestion would be to use a class instead of a 3-element list for data. It will make your code much clearer to read:

   def point_distance((p1, p2)):
      return math.sqrt((p1.x-p2.x)**2) + (p1.y-p2.y)**2)
Stephen
You need `self.dna` in the last line
David Zaslavsky
@David: Thanks, was just editing that :)
Stephen
+1  A: 

Here's a better way to write the loop in distance(): put the following function definition, taken from the itertools documentation, in your code:

from itertools import izip, tee
def pairwise(iterable):
    a, b = tee(iterable)
    next(b, None)
    return izip(a, b)

Then you can write distance to take advantage of Python's efficient iterator manipulation routines, like so:

from itertools import chain, islice
def distance(self): 
    all_pairs = islice(pairwise(chain(self.dna, self.dna)), 0, len(self.dna))
    return sum(math.sqrt((p[1]-q[1])**2 + (p[2]-q[2])**2) for p,q in all_pairs)

This should be reasonably efficient even if the dna array is very long.

David Zaslavsky
I think pairwise doesn't circularly wrap around the array (which, if I can tell from the code, is what the OP wants)
Stephen
That's why I used `chain` to basically concatenate two copies of the array. The `islice(pairwise(chain(...)))` construction will (should) iterate over `(a[0],a[1]),(a[1],a[2]),...,(a[N-1],a[0])`.
David Zaslavsky
I see. Cool. I gotta read the itertools doc :)
Stephen
Thanks. I'll have to try this out and do some performance testing. :)
Alex