How much nesting of python list comprehensions is appropriate? [closed]

Question

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.

Closed 8 years ago.

I'm a relative python beginner, now fairly comfortable with the language but still grappling over issues of what's "pythonic" and not. I was wondering what people's thoughts on this issue are.

For example, take this line of code for calculating the average cost per week of rental properties drawn from a peewee database:

    rental_avg_costperweek = sum([calcCostPerWeek(rental.price, rental.rental_freq) for rental in
                                 [LLSRental.select(LLSRental.id == this_id) for this_id in closest_rental_ids]]) \
                             / len(closest_rental_ids)

It uses nested list comprehensions, which might be confusing.

Alternatively I could stick the inner comprehension into a temporary variable:

    closest_rental_records = [LLSRental.select(LLSRental.id == this_id) for this_id in closest_rental_ids]
    rental_avg_costperweek = sum([calcCostPerWeek(rental.price, rental.rental_freq) for rental in closest_rental_records]) \
                             / len(closest_rental_ids)

This might be (a little) easier to read, but being an ex-C++ programmer I have an aversion to creating temporary variables purely for readability, as it clutters the namespace and potentially makes more work for the garbage collector.

Also I think if a reader doesn't understand the variable is purely temporary it may make the code more confusing if there are many such variables.

As such, I'm inclined to go with the first option over the second despite the guidance of "flat is better than nested"...but what do ye python veterans think?

Thanks!
gs.

Answer 1

I think both are wrong. It looks like you're doing the inner comprehension because you want to avoid recalculating LLSRental.select(). You should rarely need inner comprehensions if you're using comprehensions appropriately, because you can nest them, like

all_the_inputs = [ process_value(x) for y in all_the_stuff for x in y ]

or something. This is a nice but short post which explains this well.

Anyway. Something like

rental_avg_costperweek = 0
for this_id in closest_rental_ids:
    rental = LLSRental.select(LLSRental.id == this_id)
    rental_avg_costperweek += calcCostPerWeek(rental.price, rental.rental_freq)
rental_avg_costperweek /= len(closest_rental_ids)

seems like a much more appropriate way to do the calculation. I'm actually creating fewer objects than your code does, because I don't have two lists I make for no good reason, whereas your code makes two lists for intermediate calculations and then throws them away.