5
\$\begingroup\$

I've tried solving the following HackerRank question:

Write a function blur_image() that takes as a parameter an image in the form of a nested list A and blurs it by averaging each pixel value with four of its neighboring pixels (Top, Bottom, Right, and Left). Note: not all of the neighbors are available in boundary cases. You have to write suitable conditions accordingly.

My Solution:

import ast
A = input()
A = ast.literal_eval(A)

def blur_image(a):
    result = []
    for i in range(len(a)):
        row = []
        for j in range(len(a[i])):
            total, count = a[i][j], 1
            if i + 1 < len(a): total, count = total + a[i+1][j], count + 1 
            if j + 1 < len(a[i]): total, count = total + a[i][j+1], count + 1
            if i - 1 > -1: total, count = total + a[i-1][j], count + 1 
            if j - 1 > -1: total, count = total + a[i][j-1], count + 1
            row.append(round(total/count, 2))
        result.append(row)
    return result

print(blur_image(A))

I would appreciate any suggestions and advice you can give me to improve this solution. Please note that my focus is to solve this without using any modules.

\$\endgroup\$
  • \$\begingroup\$ Do you care for speed at the expense of longer code? \$\endgroup\$ – Cris Luengo Jan 17 at 2:04
  • \$\begingroup\$ If you want speed, look up summed area tables. \$\endgroup\$ – user1118321 Jan 17 at 2:18
3
\$\begingroup\$

First, I think this code arguably abuses tuple destructuring to declare and reassign variables. Declaring multiple variables on a line generally hurts readability, just to save a line/some keystrokes. I would write everything out fully, even if that comes with the cost of verbosity. I'd also space it out a bit:

def my_blur_image1(a):
    result = []
    for i in range(len(a)):
        row = []
        for j in range(len(a[i])):
            total = a[i][j]
            count = 1
            if i + 1 < len(a):
                total += a[i+1][j]
                count += 1

            if j + 1 < len(a[i]):
                total += a[i][j+1]
                count += 1

            if i - 1 > -1:
                total += a[i-1][j]
                count += 1

            if j - 1 > -1:
                total += a[i][j-1]
                count += 1

            row.append(round(total/count, 2))

        result.append(row)

    return result

Twice, you have something along the lines of

lst = [] # Create a list
for i in range(len(a)):
    res = # Calculate some result
    lst.append(res)

I think the logic could be broken up, and could make use of some list comprehensions. This is basically the scenario that list comprehensions (and map) are intended for. Iterating over a list to produce a new list is a very common operation.

I'm not necessarily recommending this way, but it does show an alternative, more functional way of approaching the problem. I'll say that my way ended up a fair bit slower than yours. On my machine, yours takes roughly 14 seconds for a 2000x2000 matrix, while my version takes 25 seconds unfortunately. You didn't tag performance though :D

# Returns whether or not i,j is inbound for a given a matrix
def inbounds(i, j, a):
    return 0 <= i < len(a) and \
           0 <= j < len(a[i])

# Returns the inbound pixel values on and surrounding the given point
def inbound_pixels_around(i, j, a):
    # I figured it was best to hard-code the indices instead of using several "if"s like you had done
    # That way, I can make use of looping to reduce some duplication
    # If diagonal pixels were included, this could be generated by another comprehension instead
    indices = [(i, j), (i - 1, j), (i + 1, j), (i, j - 1), (i, j + 1)]

    # Iterate over the indices.
    # Remove the ones that are  out of bounds, and use the inbound ones to index the list
    return [a[i][j] for (i, j) in indices if inbounds(i, j, a)]

def my_blur_image2(a):
    # A 3D-array. Each pixel has been replaced by a list of inbound neighbor values
    inbound_neigh_rows = [[inbound_pixels_around(i, j, a) for j in range(len(a[i]))]
                          for i in range(len(a))]

    # Then iterate ever each set of neighbors, and divide the sum of the neighbors by their length
    # This does away with needing an explicit "count" variable
    return [[sum(neighs) / len(neighs) for neighs in row]
            for row in inbound_neigh_rows]

I'm making fairly extensive use of list comprehensions here. I'm using them to filter out non-inbound cells in inbound_pixels_around using inbounds, and to generate neighbors and their average values in my_blur_image2.

test_data = [[1, 2, 3],
             [4, 5, 6],
             [7, 8 ,9]]

print(blur_image(test_data))
print(my_blur_image1(test_data))
print(my_blur_image2(test_data))

[[2.33, 2.75, 3.67], [4.25, 5.0, 5.75], [6.33, 7.25, 7.67]]
[[2.33, 2.75, 3.67], [4.25, 5.0, 5.75], [6.33, 7.25, 7.67]]
[[2.3333333333333335, 2.75, 3.6666666666666665], [4.25, 5.0, 5.75], [6.333333333333333, 7.25, 7.666666666666667]]

Updated to make use of comparison chaining. Thanks @Mathias.

\$\endgroup\$
  • \$\begingroup\$ You can simplify inbounds by using extended comparison syntax: 0 <= i < len(a) and 0 <= j < len(a[i]). \$\endgroup\$ – Mathias Ettinger Jan 17 at 8:12
  • \$\begingroup\$ @MathiasEttinger Oh yes, thanks. Forgot Python had that. I'll need to update that on a bit. \$\endgroup\$ – Carcigenicate Jan 17 at 14:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.