PEP8 convention for returning one-line for-loop

Question

Note: this is a convention question; the code does work. That said, I am polishing up an 8 mo. project and can't figure out the best way to style the following:

def foo(Xs, Ys, image):

    products = []
    for prod in product(Xs,Ys):
        products.append([prod])

    items = []
    for comb in products:
        for item in comb:
            items.append(item)

    return [[[t[1][0], t[0][0]],
            image[t[1][0]:t[1][1],
            t[0][0]:t[0][1]]] 
                for t in list(set(items))]

So I have 2 questions: 1) I know flat is better than nested, but can I return a (not-so) one-line list like this without breaking convention too much? 2) If I do want to return this beast as a pseudo-one-liner, what is the PEP8 convention regarding the spaces and brackets?

Edit: the second loop in the code was for debugging reasons and I completely forgot to take it out. Embarrassing.

It is updated to this:

def foo(Xs, Ys, image):
    products = []
    for prod in product(Xs,Ys):
        products.append(prod)

    return [[[t[1][0], t[0][0]],
            image[t[1][0]:t[1][1],
            t[0][0]:t[0][1]]] 
                for t in set(products)]

why `for t in list(set(items))` ? why not make `items` a set in the first place ... — Patrick Artner, Sep 15 '18 at 21:29
You are not returning a one-line for-loop. You are returning a list. — juanpa.arrivillaga, Sep 15 '18 at 21:32
why create products first, then iterate over it to go over its items (that are sinlge element lists created by `products.append([prod])` and stuff them in another list, then put them into a set ... — Patrick Artner, Sep 15 '18 at 21:32
It looks like you can replace your first two sets of for-loops with `items = set(item for prod in product(Xs,Ys) for item in prod)` and then replace `list(set(items))` with `items`. — Steven Rumbalski, Sep 15 '18 at 21:51

score 3 · Accepted Answer · answered Sep 15 '18 at 21:40

As far as I know, PEP8 doesn't address these long, multi-line list-comprehensions explicitly. Strictly speaking, as long as you keep the line-length down, you are "fine".

But look, I would avoid code like this like a plague. It's fun for messing around playing code-golf, but not for writing readable, maintainable code. That is the whole point of PEP8. The first thing in our tool-box for writing readable, maintainable code is to use functions.

def foo(Xs, Ys, image):

    products = []
    for prod in product(Xs,Ys):
        products.append([prod])

    items = []
    for comb in products:
        for item in comb:
            items.append(item)

    return [mogrify(t, item) for t in list(set(items))]

def mogrify(t, item):
    return [[t[1][0], t[0][0]], image[t[1][0]:t[1][1], t[0][0]:t[0][1]]]

and it's more efficient to _not_ build a list: `return [mogrify(t, item) for t in set(items)]` — Jean-François Fabre, Sep 15 '18 at 21:51

score 0 · Answer 2 · answered Sep 15 '18 at 21:35

0

As the line is over 79 charactes, I would probably indent it as such

return [
    [
        [t[1][0], t[0][0]],
        image[t[1][0]:t[1][1], t[0][0]:t[0][1]]
    ] for t in list(set(items))
]

this follows PEP8.

answered Sep 15 '18 at 21:35

dangee1705

3,445
1
21
40

why `for t in list(set(items))` not just `for t in set(items)` – Jean-François Fabre Sep 15 '18 at 21:40
2

OP didn't ask for his code to be changed, only formatted – dangee1705 Sep 15 '18 at 21:42
2

that doesn't mean you cannot suggest improvements. – Jean-François Fabre Sep 15 '18 at 21:43
I chose not to. – dangee1705 Sep 15 '18 at 21:43

Evgeny · Answer 3 · 2018-09-15T23:34:35.777

I totally side with a point that bad code cannot be cured with PEP.

mogrify() by @juanpa.arrivillaga is a great way out, but I would also consider refactoring the data structure and return a tuple of t[i][j] and use a separate constructor for slicing image.

One version of mogrify if one is averse to reading too much [][][][] in a row:

def mogrify(t, image):
    a = t[1][0]
    b = t[0][0]
    c = t[0][1]
    d = t[1][1]
    return [[a, b], image[a:d, b:c]]

Also my idea of separating constructors is:

def corners(Xs, Ys):
    # ...
    return a, b, c, d

def make_interval(a, b, c, d):
    return [a, b]

def slice_image(image, a, b, c, d):
    return image[a:d, b:c]

PEP8 convention for returning one-line for-loop

3 Answers3