3

I'm trying to convert this nested loop into a list comprehension but i'm not sure it's possible as there is different values possible for item in "tmp" list. Is it the best way to do this ? Thanks !

final = []
for a in range(-13, 1):
    for b in range(0,4):
        for c in range(6, 49):
            for d in range(48, 94):
                tmp = [0 for i in range(100)]
                for i in range(100):
                    if raw_x[i] >= b and raw_y[i] >= d:
                        tmp [i] = -1
                    if raw_x[i] <= a and raw_y[i] <= c:
                        tmp [i] = 1
                final.append(tmp)
benvc
  • 14,448
  • 4
  • 33
  • 54
TmSmth
  • 450
  • 5
  • 31

3 Answers3

3

nested comprehensions are not very readable

a simple 

[something for something in container if something > 9]

is awesome, but a nested one is often confusing

you can simply move the loops into a generator function - it will still be readable and allow for lazy iteration

def no_idea_what_this_represents():
    for a in range(-13, 1):
        for b in range(0,4):
            for c in range(6, 49):
                for d in range(48, 94):
                    tmp = [0 for i in range(100)]
                    for i in range(100):
                        if raw_x[i] >= b and raw_y[i] >= d:
                            tmp [i] = -1
                        if raw_x[i] <= a and raw_y[i] <= c:
                            tmp [i] = 1
                    yield tmp

final = [signs for signs in no_idea_what_this_represents()]

EDIT: just an opinionated addendum - this way the complicated nested loop can be named ( I named it no_idea_what_this_represents for obvious reasons ) but when a programmer sees

possible_views = [matrix for matrix in camera_matrices()]

he immediately knows what that means while something like

possible_views = [device.matrix 
                  for devices in itertools.chain(connected_devices(), buffered_devices()
                  for device in devices
                  if device.type=='camera']

makes the programmer read a lot of lines and it is not obvious what is going on

Derte Trdelnik
  • 2,656
  • 1
  • 22
  • 26
  • Although it doesn't answer the actual question (turns out OP literally did just want to learn about complex comprehension expressions) this challenge to the premise makes a *very* good point about readability. -1 and +1. – jez Jan 07 '19 at 15:55
2

Your algorithm has large time complexity. First see if this is actually what you require. Once you are sure you need a nested for loop with 4 nesting levels, you can define a function to use within your list comprehension.

In this case, notice built-in map can accept multiple iterable arguments. functools.partial allows you to define functions with pre-set arguments.

from functools import partial

def get_val(x, y, a, b, c, d):
    if x <= a and y <= c:
        return 1
    if x >= b and y >= d:
        return -1
    return 0

final = [list(map(partial(get_val, a=a, b=b, c=c, d=d), raw_x, raw_y)) \
         for a in range(-13, 1) \
         for b in range(0, 4) \
         for c in range(6, 49) \
         for d in range(48, 94)]
jpp
  • 159,742
  • 34
  • 281
  • 339
  • @tobias_k `map` handles the unpacking of the lists `raw_x` and `raw_y` into elements `x` and `y` – jez Jan 07 '19 at 15:21
  • @jez Ah, right, did not see that (and apparantly did not read the description). – tobias_k Jan 07 '19 at 15:23
  • 1
    Note that, strictly, the order of precedence of -1 and +1 should be swapped if you want to match the question. In the question, if `x <=a and y<=c` then the value +1 overwrites the previous value, even if it was previously set to -1 – jez Jan 07 '19 at 15:35
  • @jez, Thanks for pointing out. Yes, that is true, since OP is potentially overwriting values in each `if` statement (intentionally or not, it's not clear). Corrected now. – jpp Jan 07 '19 at 15:36
  • thanks for your answer, i didn't choose it as i wanted only a list comprehension without external function, but i've learned something ! thanks @jez i don't even realise it – TmSmth Jan 07 '19 at 15:41
  • 1
    @TmSmth in that case be aware that my answer replicates the question's +1 -over- -1 precedence as well – jez Jan 07 '19 at 15:42
2

This can be done in one expression although I'm not sure readability is improved:

final = [
    [
        +1 if x <= a and y <= c
        else -1 if x >= b and y >= d
        else 0
        for x, y in zip( raw_x, raw_y )
    ]
    for a in range(-13, 1)
    for b in range(0, 4)
    for c in range(6, 49)
    for d in range(48, 94)
]

Note that I've assumed you want to go through the whole of raw_x and raw_y rather than exactly 100 elements every time: the question implies 100-every-time but if the intention is really to go through the whole sequence, then it's better not to hard-code the 100 in there. If I'm wrong about the intention, you can change that inner-comprehension for loop to for i in range(100) and use raw_x[i] and raw_y[i] in the conditional expression instead of x and y.

jez
  • 14,867
  • 5
  • 37
  • 64