2

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.

Community
  • 1
  • 1
isaacbernat
  • 395
  • 1
  • 5
  • 19
  • Yes, maybe I should have mentioned – isaacbernat Jan 31 '13 at 19:39
  • Asking this question after reading the first link implies a complete misunderstanding of the first link. Yes, "most (not all) pythonistas consider [list comprehensions] more direct and clearer" **for the specific purpose of** creating a list **of results each calculated from the source elements** - not to update those source elements in-place. – Karl Knechtel Mar 27 '23 at 00:40
  • With regards to closing the question as duplicate. Here it mentions "maps" (even in the title) in addition to list comprehensions and for loops (what other question is focused on). So I don't think it's really a duplicate. Also, maps are not even mentioned as answers to the other question (maybe that should be amended as well if this question is still considered duplicate). – isaacbernat Mar 28 '23 at 09:40
  • The same reasoning applies, for obvious reasons. (In 3.x, `map` is that much worse, because the generator must be "consumed" in order for the side effects to actually occur. See also https://stackoverflow.com/questions/1303347, which incidentally came up automatically in the related questions.) – Karl Knechtel Mar 28 '23 at 09:59

2 Answers2

2
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']
Abhijit
  • 62,056
  • 18
  • 131
  • 204
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
isaacbernat
  • 395
  • 1
  • 5
  • 19