0

This code computes smallest rectangle containing a list of input rectangles:

left = min(x.left for x in rect)
bottom = min(x.bottom for x in rect)
right = max(x.right for x in rect)
top = max(x.top for x in rect)

I'm sure this can be simplified to single line statement but honestly I don't know how. Any help would be appreciated.

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
mnowotka
  • 16,430
  • 18
  • 88
  • 134
  • 1
    Note that you don't need *list*-comprehensions at all. You could just use genexps: `min(x.left for x in rect)`. This avoids creating a list object. – Bakuriu Feb 04 '14 at 10:49
  • @Bakuriu - yes, that's better (corrected) but still not DRY enough. – mnowotka Feb 04 '14 at 10:54
  • I just wanted to point out that using list-comprehensions you are wasting space. However I still don't have clear if you care for the *performance* of that loop or you simply want to avoid code duplication. – Bakuriu Feb 04 '14 at 12:26

2 Answers2

1

Don't use a generator expression here, just loop once, updating minimi and maximi in one go:

left = bottom = float('inf')
right = top = float('-inf')
for x in rect:
    if x.left < left:     left = x.left
    if x.bottom < bottom: bottom = x.bottom
    if x.right > right:   right = x.right
    if x.top > top:       top = x.top

Yes, this is more verbose, but also more efficient. The longer it is, the less work the above loop performs compared to your 4 generator expressions.

You can produce dictionaries of results:

minimi = dict.fromkeys(('left', 'bottom'), float('inf'))
maximi = dict.fromkeys(('right', 'top'), float('-inf'))
for x in rect:
    for key in minimi:
        if getattr(x, key) < minimi[key]: minimi[key] = getattr(x, key)
    for key in maximi:
        if getattr(x, key) > maximi[key]: maximi[key] = getattr(x, key)

but that's hardly worth the abstraction, not for 2 values each.

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
  • I'd prefer: `left = min(left, x.left)` etc. avoiding all the `if`s... surely you pay 4 function calls which can be expensive, but I find it more readable. – Bakuriu Feb 04 '14 at 10:48
  • @Bakuriu: That'd mean you call a function and assign 4 times every loop. Just testing is going to be faster. The OP is doing this inside a nested `for` loop, compounding the difference. – Martijn Pieters Feb 04 '14 at 10:50
  • OK, I thought there can be a single list comprehension, equivalent to one loop, but more concise... – mnowotka Feb 04 '14 at 10:51
  • @mnowotka: No, a list comprehension always produces *one* list. You can split out rows in to columns with `zip()` but you'd still end up with 4 loops to determine separate minima and maxima. – Martijn Pieters Feb 04 '14 at 10:53
  • @mnowotka, what you mean is to create 4 lists out of one comprehension... and AFAIK there is no such a thing. – bgusach Feb 04 '14 at 10:54
  • @MartijnPieters what about using ternaries to "compress" the code in a more elegant way? – bgusach Feb 04 '14 at 11:01
  • @ikaros45: You mean to test against an external variable in the list comprehension loop? *shudder*. That'll get **really, really ugly**. I much prefer explicit looping. – Martijn Pieters Feb 04 '14 at 11:04
  • @ikaros45: List comprehensions are a lovely tool, but you should not try and cram all loops into one just for the sake of using a list comprehension. We are *not* producing a list result here, only minimi and maximi. – Martijn Pieters Feb 04 '14 at 11:04
  • Have you actually timed this, though? This approach has the disadvantage that the actual looping process must be interpreted. – Karl Knechtel Feb 04 '14 at 11:28
  • @KarlKnechtel: and what makes you think the generator expression is not a loop to be interpreted? – Martijn Pieters Feb 04 '14 at 11:30
  • @KarlKnechtel: But yes, this has been timed many times before. – Martijn Pieters Feb 04 '14 at 11:30
  • @KarlKnechtel: Example: [How can I get the minimum and the maximum element of a list in python](http://stackoverflow.com/a/16938176) – Martijn Pieters Feb 04 '14 at 11:36
  • @KarlKnechtel: uhm, which actually contradicts my statements, but that test had straight lists to work with, not generator expressions. :-P. – Martijn Pieters Feb 04 '14 at 11:37
  • @MartijnPieters no, what I meant was just your first proposal, but instead of inline ifs, just ternary operators. – bgusach Feb 04 '14 at 11:42
  • @ikaros45: right, but that would still invoke an assignment for every loop for all 4 variables. For most values, my version just tests, and only rarely assigns. – Martijn Pieters Feb 04 '14 at 11:45
0

You can solve the problem with a reduction, but it's probably less efficient than Martijn Pieters suggestion.

from functools import reduce

def fn(current_mins_maxs, r):
    z = zip(current_mins_maxs, (r.left, r.bottom, r.right, r.top))
    return [min(z[0]), min(z[1]), max(z[2]), max(z[3])]

first, rest = rectangles[0], rectangles[1:]
# Or, if you are using Python 3, you can write:
# first, *rest = rectangles
mins_and_maxs = reduce(fn, rest, (first.left, first.bottom, first.right, first.top))
superjump
  • 151
  • 4