Today I was parsing a directory index with a list of paths to zip files using BeautifulSoup and came across an interesting thing. Let's assume I would like to take all the href
properties of the tags I got and put them directly into a Queue:
q = Queue.Queue()
[q.put(tag['href']) for tag in soup.findAll('a')]
I've never run into a situation like this before where a comprehension could be used inline without assigning it to anything, just to generated another iterator through some routine call. Is this considered bad practice? Is it "pythonic", per se? Was there a better one-liner to put all the items into the queue?
If you think of it as a loop over the list returned by soup.findAll it would look like this:
This is probably the more 'pythonic' form as 'explicit is better than implict'
There are many opinions on this thread, I can only speak from coding conventions at my organization.
there are many ways to affect a loop, but a key attribute of list comprehensions is that they create lists, with one item for each in the iterated over sequence.
this unused list is obviously wasteful. As such, this construction, a list comprehension with unused return value; is forbidden from appearing in our code base. An explicit loop like above, or a generated combined with something that consumes it, for instance:
or just:
is required to pass review.
Probably not a better one-liner, but I'd (personally) consider doing this instead of:
to be bad practice. Firstly, the one liner will generate a list full of
[None]
, which is likely more inefficient. Secondly, it's not immediately clear what the code is doing; seeing a list comprehension generally implies the resultant list will be used in some way.This has been asked many times, e.g., here and here. But it's an interesting question, though. List comprehensions are meant to be used for something else.
Other options include
map()
- basically the same as your samplefilter()
- if your function returns None, you will get an empty listfor
-loopwhile the plain loop is the preferable way to do it. It is semantically correct in this case, all other ways, including list comprehension, abuse concepts for their side-effect.
In Python 3.x,
map()
andfilter()
are generators and thus do nothing until you iterate over them. So we'd need, e.g., alist(map(...))
, which makes it even worse.