Question

TL;DR Which is the best?

1.- [r.update(r.pop('some_key')) for r in res if r.get('some_key')]

2.- map(lambda r: r.update(r.pop('some_key') if r.get('some_key') else []), res)

3.- map(lambda r: r.update(r.pop('some_key')), filter(lambda r: r.get('some_key'), res))

4.- for r in res:
        if r.get('some_key'):
            for element in r['some_key']:
                r[element] = r['some_key'][element]
            del r['some_key']

5.- Insert your own approach here

Note : This is not production code. It is code that is run in a test suite, so I am more concerned about legibility/maintainability than performance. Nevertheless I would also like to know if the decision regarding which is better (accounting the tradeoff performance/legibility) would change if this was production code. The number of elements 'some_key' is quite small if this makes a difference.

Context : I have read Python List Comprehension Vs. Map where the accepted answer says:

[...] List comprehensions may be faster in other cases and most (not all) pythonistas consider them more direct and clearer [...].

Nonetheless, the accepted answer to Is it Pythonic to use list comprehensions for just side effects? says:

It is very anti-Pythonic to do so [use comprehension lists for side effects only, ignoring return value], and any seasoned Pythonista will give you hell over it. The intermediate list is thrown away after it is created, and it could potentially be very, very large, and therefore expensive to create.

PS: I already have an opinion on which one is the best, but one coworker of mine disagrees. This is why I am enquiring.

Was it helpful?

Solution 2

I think that this variation on Abhijit answer, which at the same time is a variation of my 4th point is the best.

for r in res:
    r.update(r.pop('some_key', {}))
  • Has all the advantages of using a regular for loop
  • Is easy to understand
  • Takes just 2 lines of code
  • Doesn't need the if clause

OTHER TIPS

1.- [r.update(r.pop('some_key')) for r in res if r.get('some_key')]

List Comprehension for side effect is not recommended. It creates a list which is discarded at the end of the operation

2.- map(lambda r: r.update(r.pop('some_key') if r.get('some_key') else []), res)

Not much different from 1 except that, this will break in Py 3.X.

3.- map(lambda r: r.update(r.pop('some_key')), filter(lambda r: r.get('some_key'), res))

Worse then 3. Apart from using map and relying on side effect, you are adding the overhead of a function call

4.- for r in res:
        if r.get('some_key'):
            for element in r['some_key']:
                r[element] = r['some_key'][element]
            del r['some_key']

This is better than the rest

  1. No unnecessary List generated that will be discarded
  2. Will work in Py 3.x
  3. No explicit function call overhead

5.- Insert your own approach here

    for r in res:
        if 'some_key' in r:
            r.update(r['some_key'])
            del r['some_key']
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top