I'm wondering if someone could take some time to review this script. I'm parsing through a list of points of any length and calculating the distance. I'm wondering how to make my code better/more efficient (something I'm working on). A sample input file would be like so:

300.754178236262248 103.453277023380423 0,276.62980277988612 90.123295023340319 0,269.345711570634421 103.319531391674346 0,293.447811515317824 116.649513392506364 0,300.754178236262248 103.453277023380423 0

I'm not sure why the zeros are there; these are from the spacenet label csv's.

And this is my code:

import math

def calc(x1, y1, x2, y2):
    dist = math.sqrt((x2-x1)**2 + (y2-y1)**2)
    return dist 

li = []
res = []
with open("/Users/jin/points.txt") as filestream: 
    for line in filestream:
        temp = line.split(",") #splits by the comma in a single list 

        for i in temp:
            temp = i.split(" ") #splits by spaces to individual lists of points 
            li.append(temp) #list of lists containing each point 
    # for item in li: 
    #   x1 = item[1]
    #   y1 = item[0]

    # for item in li: 
    #   for pt in item: 
    #       print pt

    for index in range(len(li)-1): 
        one = li[index]
        two = li[index+1]
        x1 = float(one[0])
        y1 = float(one[1])
        x2 = float(two[0])
        y2 = float(two[1])
        res.append(calc(x1, y1, x2, y2))
    print res

2 Answers 2

  1. Choose a better name than calc(). Something like distance() or euclidean_distance().

  2. There is no need for the variable temp, just write for i in line.split(",")

  3. The for i in temp loop can be written more simply as:

    li.extend(i.split() for i in temp)
  4. Remove all commented out code that you don't immediately need.

  5. The for index in range(len(li)-1): doesn't need to be in the with statement, as it is not using filestream.

  6. res can be calculated as part of the for i in temp: loop and avoid li altogether.


You can make the calculation of the distances a bit easier by using numpy, which allows you to perform calculations on the whole element at once or only on one dimension.

Also, your reading of the input can be a bit simplified using list comprehensions:

import numpy as np

def read_file(file_name):
    points = []
    with open(file_name) as f:
        for line in f:
            points.extend([float(value) for value in point.split()[:2]]
                          for point in line.split(","))
            # alternative functional approach:
            # points.extend(list(map(float, point.split()[:2]))
            #               for point in line.split(","))
    return np.array(points)

def distance(p1, p2):
    return np.sqrt(((p2 - p1)**2).sum(axis=1))

if __name__ == "__main__":
    points = read_file("/Users/jin/points.txt")
    print(distance(points[:-1], points[1:]))

Note the indexing of x to get two arrays, one with all points except the last and one with all points except the first.

The nice thing about this distance function is that it works with N-dimensional points. So if you need to upgrade to 3D you don't need to change anything. And you can actually already use this, since your list might actually contain 3D points (with the z-value always being 0). So you can get rid of the [:2] in my read_file function.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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