Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have this short Python script for making images brighter:

import skimage.io
import math
import sys


def square_root_filter(input_image_file_name, output_image_file_name):
        image_data = skimage.io.imread(input_image_file_name)
        image_data_height = len(image_data)
        image_data_width  = len(image_data[0])

        for row_index in range(image_data_height):
                for column_index in range(image_data_width):
                        channel = image_data[row_index][column_index]

                        r = float(channel[0])
                        g = float(channel[1])
                        b = float(channel[2])

                        r /= 255.0
                        g /= 255.0
                        b /= 255.0

                        r = math.sqrt(r)
                        g = math.sqrt(g)
                        b = math.sqrt(b)

                        channel[0] = int(255 * r)
                        channel[1] = int(255 * g)
                        channel[2] = int(255 * b)

        skimage.io.imsave(output_image_file_name, image_data)


def main():
        square_root_filter(sys.argv[1], sys.argv[2])


if __name__ == "__main__":
        main()

Output

enter image description here

enter image description here

(I tested it only with Python 3.5. Also, it will take some time to do its work.)

I am not a professional Python programmer, so please tell me anything that will make the code better.

share|improve this question
up vote 10 down vote accepted

PEP 8 specifies four spaces per level of indentation. Since whitespace matters in Python, this is a pretty strong convention.

You should round() the results to the nearest integer rather than truncating them towards 0.

Each of the three channels is treated identically and independently, so you shouldn't write the same code three times.

def square_root_filter(input_image_file_name, output_image_file_name):
    image_data = skimage.io.imread(input_image_file_name)
    for row_data in image_data:
        for pixel in row_data:
            for channel in range(3):
                pixel[channel] = round(255 * math.sqrt(pixel[channel] / 255))
    skimage.io.imsave(output_image_file_name, image_data)

If you take advantage of the fact that channels is a NumPy array, you can vectorize the calculation.

import numpy as np
import skimage.io

def square_root_filter(input_image_file_name, output_image_file_name):
    image_data = skimage.io.imread(input_image_file_name)
    for row_data in image_data:
        for pixel in row_data:
            pixel[:] = np.rint(255 * (pixel / 255) ** 0.5)
    skimage.io.imsave(output_image_file_name, image_data)
share|improve this answer
2  
255 is the right value because the maximum value of a pixel is 255 (0 to 255 is 256 values). So, dividing by 255 makes the maximum value 1. – Mark H 12 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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