0

I have a code with all this so far:

import math

nList = [[[0,0,0],[3,2,1]],\
[[]],\
[[1,1,12]],\
[[0,0,0],[30000,40,3010],[32000,40500,7520],[0,0,10520]],\
[[15340,0,0],[104300,0,3630], [434000,4434000,63460],[400340, 404300, 9430],[10340,0,1334320]]]



MaxLengthPerTrace=[[]]

traceCount = len(nList)
prevX = 0
prevY = 0
prevT = 0
maxLengthTempList=0

def calculateLength(x1, y1, x2, y2):
    return math.sqrt((x1-x2)**2 + (y1 - y2)**2)
def calculateTime (t1,t2):
    return abs(t1-t2)

#Entry point
for traceIndex in range(0, len(nList)):
    print 'trace: ' + str(traceIndex+1)

    trace = nList[traceIndex]
    for pointIndex in range(0, len(trace)):
        point = trace[pointIndex]

        if len(point)>0:
            tempX = point[0]
            tempY = point[1]
            tempT = point[2]

            if pointIndex != 0:

                #Calulate Length here
                length = calculateLength(tempX, tempY, prevX, prevY)
                print length


                #Calculate Maximum Length here

How would I write a program to calculate a maximum length from length? I wrote pseudo-code for this as

        if(length > max)
            max = length;   

        add the length in MaxLengthPerTrace in index of traceindex

but can't seem to execute it in Python 2.7.10 properly. Any help would be appreciated. Thanks!

I tried this but it didn't work, coming with error: "for i in length: TypeError: 'float' object is not iterable"

for i in length:
                if i >length:
                    i=length
                    MaxLengthPerTrace.append(i)
                    print MaxLengthPerTrace
Jayden
  • 65
  • 2
  • 9
  • Max would be the maximum length from the lengths identified within function: length => def calculateLength(x1, y1, x2, y2): return math.sqrt((x1-x2)**2 + (y1 - y2)**2) ... calculateLength(tempX, tempY, prevX, prevY) – Jayden Apr 11 '15 at 07:38
  • How do you represent a point in the space? With only two coordinate or with three coordinate? – Alessandro Suglia Apr 11 '15 at 07:52
  • A point is three coordinates so [0,0,0] in 1st one for example. – Jayden Apr 11 '15 at 08:02
  • 1
    As a quick note, you don't need to use `\\` for line continuation inside list literals - the brackets imply line continuation until they are closed in Python. – Gareth Latty Apr 11 '15 at 08:03

3 Answers3

2

Your code seems to have a couple of issues, so I'm going to have to make a few guesses, but hopefully this helps.

This is a good case for a generator. You have an iterable (the traces), and want to do something reasonably complicated to create a new iterable (the lengths):

nList = [[[0, 0, 0], [3, 2, 1]],
         [[]],
         [[1, 1, 12]],
         [[0, 0, 0], [30000, 40, 3010], [32000, 40500, 7520], [0, 0, 10520]],
         [[15340, 0, 0], [104300, 0, 3630], [434000, 4434000, 63460],
          [400340, 404300, 9430], [10340, 0, 1334320]]]

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

def calculate_time (t1,t2):
    return abs(t1-t2)

def lengths(traces):
    previous_x, previous_y = 0, 0
    for trace in traces:
        for index, point in enumerate(trace):
            if point:
                x, y, t = point
                if index > 0:
                    yield calculate_length(x, y, previous_x, previous_y)
                previous_x, previous_y = x, y  # Was this intended?

print(max(lengths(nList)))

In your code, you never set prevX and prevY, which I assume is a bug. I've given a guess at what you want here.

Note the changes I have made - firstly, iterating by index in Python is slow, unreadable, and generally never a good idea - Python for loops iterate directly over iterables, no need to count. If you need the index (as in the second case to check if it's not the first value), we can use enumerate() to get that as well.

Once we have an iterable of the lengths, it's trivial to obtain the maximal length using the max() built-in.

If you wanted the maximum length per-trace, rather than over the entire data set, you can simple change the level the function works at (remove the outermost loop and change the input argument to trace), and then call it inside a list comprehension, for example:

def lengths(trace):
    previous_x, previous_y = 0, 0
    for index, point in enumerate(trace):
        if point:
            x, y, t = point
            if index > 0:
                yield calculate_length(x, y, previous_x, previous_y)
            previous_x, previous_y = x, y

print([max(lengths(trace)) for trace in nList])

As a final note, I also changed the code style to match PEP8 - this is not vital, but it helps make your code consistent with most examples you will find and makes it easier to recognise things at a glance. Mostly this just means I've renamed variables.

Community
  • 1
  • 1
Gareth Latty
  • 77,968
  • 15
  • 168
  • 174
  • I set prevX and prevY to 0 towards the top part. – Jayden Apr 11 '15 at 08:03
  • @sayna Indeed, I didn't word that well - what I mean is they are always `0`, which I assumed wasn't intended as the names implied they were meant to be the previous point (which would explain their involvement in calculating a length - from point to point), if that's not the intention, you'll need to explain what you are looking for. – Gareth Latty Apr 11 '15 at 08:05
  • you got it spot on. New to programming so building up one block at a time! – Jayden Apr 11 '15 at 08:06
  • @sayna Great. I may have introduced a lot of concepts in my post, but hopefully you can see that they make the code much easier to understand once you know about those language features. If you break down my post and look at the parts, it should really help you write more Pythonic code (which should mean it's easier to work with for you). – Gareth Latty Apr 11 '15 at 08:16
  • Hi @Lattywire, thanks for all the help. When I try to find max length per trace: def length(traces): previous_x, previous_y = 0, 0 for trace in traces: for index, point in enumerate(trace): if point: x, y, t = point if index > 0: yield calculate_length(x, y, previous_x, previous_y) ## previous_x, previous_y = x, y # Was this intended? print([max(length(trace)) for trace in nList]) I get an error saying "x, y, t = point TypeError: 'int' object is not iterable" – Jayden Apr 11 '15 at 08:40
  • @sayna Are you passing in `nList` or an element *from* `nList`? The error implies that it's iterating down too many levels and trying to make a point from a number inside a point. If you want to get the maximum per-trace, you will need to remove the outermost loop and change the method argument to `trace` as I mentioned. – Gareth Latty Apr 11 '15 at 08:42
  • I know this is a very nooby question but which outer oop do i remove? – Jayden Apr 11 '15 at 08:56
  • @sayna I edited the answer to show the example. The outer loop just means the first one in the function. – Gareth Latty Apr 11 '15 at 08:59
  • Hi Lattywire, thanks again but if I try to run it, it is giving the error: "ValueError: max() arg is an empty sequence" – Jayden Apr 11 '15 at 11:25
  • @sayna That means you are not getting any results from your generator. This will be because of your second value where the are no points. You can stop it by adding a guard to your list comprehension, to exclude traces with no lines. e.g: `print([max(lengths(trace)) for trace in nList if len(trace) > 1])`. When you come across errors like this, try printing out the individual parts of the program - see where the values you get don't match with what you get, and try to work out why. – Gareth Latty Apr 11 '15 at 12:08
  • Thanks @Lattyware! You're a lifesaver. – Jayden Apr 11 '15 at 12:20
  • Hi again @Lattyware. Is there a way to store the results of yield calculate_length(x, y, previous_x, previous_y) in a list? – Jayden Apr 11 '15 at 12:45
  • @sayna A generator is just a type of iterable, so to make it a list you can use the `list()` built-in function, as with any iterable. i.e.: `list(lengths(values))`. – Gareth Latty Apr 11 '15 at 12:48
  • Hi @Lattyware. Could you please tell me how to print the output of all the lengths as well, not just the maximum? – Jayden Apr 12 '15 at 10:01
  • @sayna If you have a list of the lengths (as above), you can loop over it and print the values, and then get the maximum of it with `max()`. If you have more questions, please post them as actual questions rather than comments on this answer. – Gareth Latty Apr 12 '15 at 14:17
  • Thanks @Lattyware. I did. Appreciate all the help! – Jayden Apr 13 '15 at 03:52
2

I changed your code a little bit just to clear the point where you get stuck. But this way of writing program is very unclear. I strongly recommend to enhance functional solutions of other folks here instead of improving your C-like style code.

import math

nList = [[[0,0,0],[3,2,1]],\
[[]],\
[[1,1,12]],\
[[0,0,0],[30000,40,3010],[32000,40500,7520],[0,0,10520]],\
[[15340,0,0],[104300,0,3630], [434000,4434000,63460],[400340, 404300, 9430],[10340,0,1334320]]]



MaxLengthPerTrace=[[]]

traceCount = len(nList)
prevX = 0
prevY = 0
prevT = 0
maxLengthTempList=0

def calculateLength(x1, y1, x2, y2):
    return math.sqrt((x1-x2)**2 + (y1 - y2)**2)
def calculateTime (t1,t2):
    return abs(t1-t2)

#Entry point
def getLengths():
    for traceIndex in range(0, len(nList)):
        print 'trace: ' + str(traceIndex+1)

        trace = nList[traceIndex]
        def getLenthsForElement():
            for pointIndex in range(0, len(trace)):
                point = trace[pointIndex]

                if len(point)>0:
                    tempX = point[0]
                    tempY = point[1]
                    tempT = point[2]

                    if pointIndex != 0:

                        #Calulate Length here
                        length = calculateLength(tempX, tempY, prevX, prevY)
                        yield length
        yield [l for l in getLenthsForElement()]                

#print the list of lists of weights
print [l for l in getLengths()]
#flatten all lists and get maximum.
lens = []
[lens.extend(l) for l in getLengths()]
print max(lens)
Nikita
  • 4,185
  • 2
  • 21
  • 44
  • 1
    `[lens.extend(l) for l in getLengths()]` - using a list comprehension for side effects is a really bad practice - either use a normal `for` loop, or better yet, use `itertools.chain.from_iterable()` to flatten here. – Gareth Latty Apr 11 '15 at 08:12
  • 1
    You're completely right. I was not sure that `lens = [i for l in getLengths() for i in l]` is clear for beginner and implement fast and dirty approach. It's anyway have side effects in getLentgh() so i would leave my answer as it is. – Nikita Apr 11 '15 at 08:30
  • 1
    I wouldn't recommend that approach for flattening either. `itertools.chain.from_iterable(getLengths())` is the best way to flatten your iterable one level. It will be faster than any method relying on pure-python loops. In a similar vein, using `list(getLengthsForElement())` would be better than a no-op list comprehension if you feel you need a list. – Gareth Latty Apr 11 '15 at 08:33
1

Make a list of the lengths, then get the maximum:

lengths = []
for point in trace:
    lengths.append(calculateLength(point[0], point[1], prevX, prevY));

maxLength = max(lengths)
print maxLength
Brian L
  • 3,121
  • 1
  • 12
  • 14