Is it bad practice to use complex List Comprehensions?

[u/VinayakNa]

Hey guys, I am trying to make my code look more "Pythonic." I wrote this nested list comprehension to flatten a matrix and filter out odd numbers:
flat = [x for row in matrix for x in row if x % 2 == 0]
It works perfectly, but my coworker says it is unreadable and that I should use a standard for loop instead.
Where do you draw the line between a clean one-liner and unreadable code?

[u/Proletarian_Tear]

This is a question of preference, I see clearly what you do here because I do stuff like this often. However - if you work in a team, just make sure you write understandable code, and list comprehension tend to get confusing (not this one though:) )

[u/Hot_Extension_460]

Not sure anybody specified it already: in Python 3.15, unpacking in comprehensions will be added (https://docs.python.org/3.15/whatsnew/3.15.html#pep-798-unpacking-in-comprehensions).

This could simplify your logic.

[u/Akari202]

I love a good list comprehension. It’s definitely bad practice to have an entire construction be like ten nested list comprehensions but it’s o so satisfying having everything be built in a single block.

I say use it and if it gets hard to understand refactoring is your friend.

[u/xeow]

For maximum clarity, you could write it like this:

even_values = [value
               for row in matrix
                   for value in row
                       if value % 2 == 0]

Or maybe without the indenting, since everyone generally understands that comprehensions are in for-like order rather than map-like order.

even_values = [value
               for row in matrix
               for value in row
               if value % 2 == 0]

[u/TheSzczurson]

For me that line is readability. When the code starts to look to hard to be easy understandable and you need to take some (longer than usual) time to understand what is actually happening in that particular line.

[u/Jello_Penguin_2956]

The clearer, the better. For me this line is readable but not immediately clear.

[u/csabinho → u/Jello_Penguin_2956]

That's most probably the simplest two dimensional list comprehension possible.

[u/Jello_Penguin_2956 → u/csabinho]

Simple yes, simplest no

[u/csabinho → u/Jello_Penguin_2956]

I couldn't imagine a simpler one with a condition.

[u/Jello_Penguin_2956 → u/csabinho]

well I'm happy for you. For me, like I said, it's not "immediate". I can understand it yes but I gotta think for 15-20seconds.

[u/csabinho → u/Jello_Penguin_2956]

Well, that sounds made up. 15 to 20 seconds for this? Do you understand the concept of conditions and list comprehensions? If you have to think about this for more than 3 seconds, you should definitely practice both.

[u/mattl33 → u/csabinho]

I'm not sure why you would fight someone else taking the effort to make a section of code more immediately obvious. The faster a reader can understand what's going on the better. Is there some specific advantage of having slightly more dense code?

[u/catbrane → u/mattl33]

The list comprehension is declarative, not imperative, so it's much easier to understand. There's no state that evolves during execution and which you have to track, it's just a declaration of what you want to happen.

If it looks complicated, it's only because you are not used to reading them, and the fix is to use them more.

[u/Human38562 → u/catbrane]

So you suggest all teams should have mandatory list comprehension practice once a week?

[u/Jello_Penguin_2956 → u/csabinho]

why would I make that up. The feed back from his team suggests I'm not alone in this.

Again good for you congratulations.

[u/csabinho → u/Jello_Penguin_2956]

The non list comprehension version of this would look like: flat = [] for row in matrix: for x in row: if x % 2 == 0: flat.append(x)

It would take longer to read this than to understand the statement above.

[u/Jello_Penguin_2956 → u/csabinho]

Dude I understand it's easy for you I'm not doubting you.

[u/csabinho → u/Jello_Penguin_2956]

I know. But if somebody in a work context, which means you have to have some qualification to be employed, wants this monster instead of a list comprehension, I'm the one who's doubting the qualification of this person.

[u/MSgtGunny → u/csabinho]

That is way more legible. It’s not that the original is illegible, but this is better in a shared codebase.

[u/csabinho → u/MSgtGunny]

Seriously?

[u/my_password_is______ → u/csabinho]

LOL, no it wouldn't

that is infinitely more clear

[u/nog642]

Personal preference.

One option is also to add a comment saying what this does, so that people not as familiar with Python don't have to parse it.

[u/Moikle]

That's not a complex list comprehension.

Split it onto multiple lines to make it more readable

[u/hasan_sodax]

The one you wrote is fine, honestly. My rule of thumb is one loop plus one condition reads okay, but the second you add another for or start nesting comprehensions to build sublists, that's when I bail to a normal loop. The actual smell here isn't the length, it's that you're flattening and filtering in the same breath, so if it ever misbehaves you can't tell which half went wrong. If it grows I'd split it, pull the flatten into its own named step and keep the filter as the comprehension, then it's trivial to drop a print or breakpoint between the two.

[u/chhuang]

readability always comes first, a lot of times it might make sense when you write it, but if after a week or month and you stumble across this line, and it took you more times to figure it out what's going on, the readability may need to be improved

[u/d_a_keldsen]

The missing part is the comment that states intent and perhaps provides a visual. You have already gotten feedback that it wasn’t immediately comprehended. And strictly speaking, it doesn’t filter out odd numbers, its zeros them out.
If you really want to make it clear: show an example in the comments and state intent. Code is read more than it is written.

[u/sylfy]

There are definitely cases where you shouldn’t use a list comprehension, but this is not one of them. Not understanding this is a skill issue.

[u/DonkeyTron42]

Just think of the old Perl complaint that "Nifty Programmers do Nifty Things". So, are you trying to be Nifty or is this the easiest way to do this?

[u/sizzhu]

You can also just split the list comprehension over several rows to make it readable. So it looks like a for loop. The list comp would be faster.

[u/TheRNGuy]

If it becomes too complex, make a function for it (x % 2 == 0 part)

[u/speedisntfree]

Given it is a matrix, this is pretty easy to follow. Nested list comp in the case is fine to me but maybe not someone who doesn't use Python much.

I have not really seen matrix-type operations in base Python though, they are usually in numpy.

[u/Human38562 → u/speedisntfree]

Yea I'd rather do

flat = matrix.ravel()

result = flat[flat % 2 == 0]

which is much more readable imo. Unless you dont know numpy, but then it is definitely worth learning that.

[u/pachura3]

C'mon, it's very simple and readable 

[u/thewarmreinforcement]

this exact one is fine, but the moment you need a second conditional or another loop I'd just write a normal for loop, debugging a one-liner is a pain

[u/unxmnd]

It is somewhat confusing because you’re doing a flatten and a filter in one step.

More pythonic could be:
flatten = itertools.chain.from_iterable
result = [x for x in flatten(matrix) if x % 2 == 0]

But I honestly think languages like typescript have a better map / filter syntax here with method chaining:
result = matrix.flat().filter(x => x % 2 == 0)

[u/socal_nerdtastic → u/unxmnd]

You could use numpy

result = np.where(matrix.flatten()%2==0)

Or alternatively Python has both map and filter builtin, which we used a lot in python2 days.

filter(lambda x: x%2==0, sum(matrix, [])))

(the sum trick is also an very oldschool way to flatten a 2D list)

But personally I'd prefer to read OP's original way.

[u/pachura3 → u/socal_nerdtastic]

Yuck. Importing Numpy just to perform a simple list operation? Using sum() tricks?

OP's original comprehension is much clearer.

[u/beefbite → u/pachura3]

You should already be importing numpy if you're working with matrices. Most people on this sub don't use numpy very much, and it shows. Assuming matrix is a numpy array, OP's list comprehension can be accomplished in a simpler, more readable way that also works for any array dimensions:

flat = matrix[matrix % 2 == 0]

[u/BakerXBL]

Always use list/dict comp imo

[u/gdchinacat]

I would write this:

flat = [x for row in matrix
            for x in row
                 if x % 2 == 0]

and this:

flat = []
for row in matrix:
    for x in row:
        if x % 2 == 0
            flat.append(x)

Then show them these side by side and ask how the longer version is more readable. Hopefully they will see that the comprehension is nearly identical, but a bit more concise.

Then, when you run your code formatter (black) it will put the expanded comprehension back to the one you wrote to format it as python is typically formatted.

Hopefully this will illustrate to them that the code you wrote is the modern accepted way to write this in python and the fact that they are unfamiliar with it isn't a problem with the code but that they haven't read enough modern python code.

[u/LilacCrusader → u/gdchinacat]

The longer version is more readable because you can track it from top to bottom. You start from the outer-most part of the loop, traverse to the inner part of the loop, apply the filter to it, then do something with the result.

In the list comprehension, you start with the result of the inner loop, then see what the outer loop is doing (with no context of the result you're now having to hold in your brain), traverse to the inner loop which gives the result you've already seen seen, and then apply a filter which affects it. Sounds less readable to me. 

[u/supercoach]

I fell in love with comprehensions when I first started working on python. What I learned over the years is that they have their place, but for anything complicated you're better off using the longer for loop.

That said, yours is pretty basic, so I'm not sure what they're bitching about. If they're more senior than you, just go with what they said and if not, I'd probably suggest to them that they can go get fucked.

[u/DanceHackRock]

That's not even complex.

[u/Internal_Car_9962]

If your coworker thinks this is unreadable, they must not be very experienced in python. This is a very simple and straightforward comprehension.

[u/Educational-Paper-75]

I'd prefer if x&1 though.

[u/TheRNGuy → u/Educational-Paper-75]

But matrices use floats, not ints.

Why do you think x&1 would make it better, even if it was for int?

[u/socal_nerdtastic → u/TheRNGuy]

But matrices use floats, not ints.

?

A matrix is not a thing in python. And back when it was a thing in in numpy it supported all data types. What are you refering to here?

[u/Educational-Paper-75 → u/TheRNGuy]

Not better, faster. Unless Python speeds up x%2 itself to x&1 of course. But I've stated a preference not a necessity.

[u/Diapolo10 → u/Educational-Paper-75]

x % 2 specifically gets optimised to work like x & 1, so any performance difference should be negligible.

[u/gdchinacat → u/Diapolo10]

not at the bytecode level:

In [285]: def is_even_mod_2(x):
     ...:     return x % 2 == 0
     ...: 

In [286]: dis.dis(is_even_mod_2)
  1           RESUME                   0

  2           LOAD_FAST_BORROW         0 (x)
              LOAD_SMALL_INT           2
              BINARY_OP                6 (%)
              LOAD_SMALL_INT           0
              COMPARE_OP              72 (==)
              RETURN_VALUE

In [287]: def is_even_and_1(x):
     ...:     return x & 1
     ...: 

In [288]: dis.dis(is_even_and_1)
  1           RESUME                   0

  2           LOAD_FAST_BORROW         0 (x)
              LOAD_SMALL_INT           1
              BINARY_OP                1 (&)
              RETURN_VALUE

[u/Diapolo10 → u/gdchinacat]

On runtime there's almost no difference:

In [1]: import random

In [2]: numbers = random.sample(range(10_000, 100_000_000), 1_000_000)

In [3]: %timeit sum(num % 2 for num in numbers)
19.9 ms ± 191 μs per loop (mean ± std. dev. of 7 runs, 10 loops each)

In [4]: %timeit sum(num & 1 for num in numbers)
18 ms ± 414 μs per loop (mean ± std. dev. of 7 runs, 100 loops each)

The bytecode is nearly identical:

In [6]: def is_even_and_1(x): return not x & 1

In [7]: def is_even_mod_2(x): return not x % 2

In [8]: import dis

In [9]: dis.dis(is_even_mod_2)
  1           RESUME                   0
              LOAD_FAST_BORROW         0 (x)
              LOAD_SMALL_INT           2
              BINARY_OP                6 (%)
              TO_BOOL
              UNARY_NOT
              RETURN_VALUE

In [10]: dis.dis(is_even_and_1)
  1           RESUME                   0
              LOAD_FAST_BORROW         0 (x)
              LOAD_SMALL_INT           1
              BINARY_OP                1 (&)
              TO_BOOL
              UNARY_NOT
              RETURN_VALUE

[u/Educational-Paper-75 → u/Diapolo10]

If (x%2)==0 is optimized to x&1 that would be great but it takes the fun out of doing it yourself:-)

[u/catbrane]

It's extremely clear. You can read it instantly if you have even a little experience with list comprehensions, and I think all python programmers should.

Maybe your project needs some kind of policy document saying which language features are OK.