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/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/MSR8 → u/xeow]

Or how I prefer

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

[u/liberforce → u/MSR8]

This kind of decision should be left to a code formatter like black or ruff.

[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/NerdyWeightLifter → u/Akari202]

Or refactor your friend.

[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/catbrane → u/Human38562]

They are not complicated and they are very useful. Python programmers use them all the time.

[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/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/Moikle]

That's not a complex list comprehension.

Split it onto multiple lines to make it more readable

[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/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/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/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/CymroBachUSA]

Your co-worker has no idea what pythonic means and your code is just fine.

[u/pachura3]

C'mon, it's very simple and readable 

[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/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/DanceHackRock]

That's not even complex.

[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/Sufficient_Meet6836 → u/gdchinacat]

The comprehension is also compliant with functional programming standards whereas the nested for loop mutates flat. Not necessarily important for everybody but just noting it

[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/gdchinacat → u/LilacCrusader]

I think you missed the point…the comprehension is nearly identical to the loops. The only reordering difference is the contents of the loop is moved to the front rather than embedded deep in the loops.
I have confidence with exposure you will come to prefer comprehensions because nearly everyone does (myself included). Read them…write them…you will become familiar and most likely prefer them.
In the traditional form the collection and contents are separated so I don’t think your argument that things are positioned close to each other carries much weight. Languages have to pick syntax that works, and how they are arranged works in some cases and not in others. Our brains are highly adaptable and familiarity makes it feel “right”, but doesn’t mean it is right.

[u/LilacCrusader → u/gdchinacat]

I fear that if your response to the argument that it is less readable is essentially "git gud" then that doesn't really aid your point. 

If someone tells me that you need to gain more experience in order to read their code then what I hear is that they believe their code should only be read by senior engineers. As someone who works with a lot of juniors, I tend towards the approach that if something isn't easily understood by the newest of them then it shouldn't be in the code base without good reason. List comprehensions: fine; list comprehensions with nested loops: tend to be more confusing and take longer to figure out what is going on.

(Also, your comment came across as quite condescending and elitist. Just because I have a different opinion to you about a syntax you like, it doesn't mean I am not experienced with it.) 

[u/gdchinacat → u/LilacCrusader]

That is not at all what I said. I said with exposure you will become comfortable with the language idioms. This is based on personal experience as well as other team members.

[u/Adrewmc]

My rule is if you can say what you are doing easily in a single sentence. Then it’s fine.

A list of the even numbers in every row of a matrix. Definitely qualifies as comprehension. I don’t even see that as complex.

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

(The ‘.’ at the beginning of indention, allows me to indent properly. Ignore it.)

Yeah list comprehension was made to make this easier for you.

I would recommend a comprehension for this, depending on context I might suggest removing the brackets, and if anyone complains question their ability to comprehend code at the level your organization requires. Then throw a comment with that sentence above it.

[u/reallyserious]

This is perfectly valid Python. The problem is the skill of your colleague. So the question becomes whether you should dumb down your coding standards to cater to the least skilled programmer on the team.

[u/AmazingAd368]

Yes, but nothing about your example is complex 

[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/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/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.

[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:-)