3

I created a line that appends an object to a list in the following manner

>>> foo = list()
>>> def sum(a, b):
...      c = a+b; return c
...
>>> bar_list = [9,8,7,6,5,4,3,2,1,0]
>>> [foo.append(sum(i,x)) for i, x in enumerate(bar_list)]
[None, None, None, None, None, None, None, None, None, None]
>>> foo
[9, 9, 9, 9, 9, 9, 9, 9, 9, 9]
>>>

The line 

[foo.append(sum(i,x)) for i, x in enumerate(bar_list)]

would give a pylint W1060 Expression is assigned to nothing, but since I am already using the foo list to append the values I don't need to assing the List Comprehension line to something.

My questions is more of a matter of programming correctness

Should I drop list comprehension and just use a simple for expression?

>>> for i, x in enumerate(bar_list):
...      foo.append(sum(i,x))

or is there a correct way to use both list comprehension an assign to nothing?

Answer

Thank you @user2387370, @kindall and @Martijn Pieters. For the rest of the comments I use append because I'm not using a list(), I'm not using i+x because this is just a simplified example.

I left it as the following:

histogramsCtr = hist_impl.HistogramsContainer()
for index, tupl in enumerate(local_ranges_per_histogram_list):
    histogramsCtr.append(doSubHistogramData(index, tupl))
return histogramsCtr
Improve this question
3
  • I realize this is just an example, but why append at all? Why not just foo = [sum(i, x) ...]? Commented Jul 30, 2013 at 20:57
  • sum(i, x) doesn't work. Do you mean i + x? Commented Jul 30, 2013 at 20:58
  • If you really want a 1-liner, you can put a for loop on one line. Not recommended, though. Commented Jul 30, 2013 at 21:06

3 Answers 3

Reset to default
9

Yes, this is bad style. A list comprehension is to build a list. You're building a list full of Nones and then throwing it away. Your actual desired result is a side effect of this effort.

Why not define foo using the list comprehension in the first place?

foo = [sum(i,x) for i, x in enumerate(bar_list)]

If it is not to be a list but some other container class, as you mentioned in a comment on another answer, write that class to accept an iterable in its constructor (or, if it's not your code, subclass it to do so), then pass it a generator expression:

foo = MyContainer(sum(i, x) for i, x in enumerate(bar_list))

If foo already has some value and you wish to append new items:

foo.extend(sum(i,x) for i, x in enumerate(bar_list))

If you really want to use append() and don't want to use a for loop for some reason then you can use this construction; the generator expression will at least avoid wasting memory and CPU cycles on a list you don't want:

any(foo.append(sum(i, x)) for i, x in enumerate(bar_list))

But this is a good deal less clear than a regular for loop, and there's still some extra work being done: any is testing the return value of foo.append() on each iteration. You can write a function to consume the iterator and eliminate that check; the fastest way uses a zero-length collections.deque:

from collections import deque
do = deque([], maxlen=0).extend

do(foo.append(sum(i, x)) for i, x in enumerate(bar_list))

This is actually fairly readable, but I believe it's not actually any faster than any() and requires an extra import. However, either do() or any() is a little faster than a for loop, if that is a concern.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

7

I think it's generally frowned upon to use list comprehensions just for side-effects, so I would say a for loop is better in this case.

But in any case, couldn't you just do foo = [sum(i,x) for i, x in enumerate(bar_list)]?

Improve this answer

4 Comments

I can't since in my program foo is a container object, the append method does some other things.
Then either just use a for loop, or, if the container object is of a class you defined, maybe change the __init__ to use *args so you can do foo = MyContainerFoo(sum(i,x) for i, x in enumerate(bar_list))
That doesn't require the use of *args. The generator expression is one argument.
Sorry, I meant I would prefer to define it so it can be initialized as either MyClass(1,2,3,4,5) (using *args) or MyClass(*(i for i in range(1,6))), rather than MyClass([1,2,3,4,5]) or MyClass(i for i in range(1,6), but thats just me.
3

You should definitely drop the list comprehension. End of.

  • You are confusing anyone reading your code. You are building a list for the side-effects.
  • You are paying CPU cycles and memory for building a list you are discarding again.

In your simplified case, you are overlooking the fact you could have used a list comprehension directly:

[sum(i,x) for i, x in enumerate(bar_list)]
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.