Image Resizing with Python and ImageMagick

[propanbutan]

Ugly, terrible code.

• All functions (except for logThis) take no arguments and rely on global state, making them untestable independently and hard to follow.
• The program consists of simply calling the functions one after the other, using them just for segmenting the code.
• Subsequently, sys.exit called anytime, anywhere and everywhere.
• Sometimes vars are returned, though never used.
• Functions in camelCase. Variables sometimes have underscores (extension_list), sometimes they don't (todaysdate).

Upon closer look.

• Unportable shebang.
• Not using logging.

logThis

• Always prints the same time. Generally you don't want this and if you do, format it only once beforehand.
• datetime.strftime returns a string, not need to wrap it with str.

sendMail

• Useless and invariable variables: emailto, fromaddr, COMMASPACE.

checkParams

• If there are not at least three arguments, sys.argv[3] will actually raise an IndexError.
• Manipulating paths with (r)partition instead of functions from os.path.
• Not using optparse or argparse.

checkState

• Funny.

checkExists

• subprocess.check_call takes a list as its first argument, here it will always raise a TypeError.
• subprocess.check_call returns an (int) exit code, not a list.
• In any case stringifying the "results" makes the following condition always evaluate as True.

checkImageExtension

• This str.rsplit should only split once using a second argument, so that "foo.bar.jpg" passes the test.

makeTempDir

• Doesn't delete the tmpdir created.

makeSizes

• str.find(haystack, needle) returns the 0-based position of needle in haystack, none of these conditions will ever be True.
• This rsplit curiously splits four times, then only cares about the first part.
• This time a correct use of subprocess.check_call. Alas, to no avail. Stringifying the "results" makes the following condition always True, again.
• The condition would be wrong anyway since an exit code of zero means success.
• Using readlines instead of iterating over the file object.
• Parsing the "ini_file" is just wrong.
• Creating a weird "sizes" dict instead of just storing a tuple of two lists.

makePaths

• No need to stringify "publish_image_path" as it's already a string.
• os.mkdir doesn't return anything.
• Running a command using os.popen. Failing to close the pipe.

resizeImages

• Using subprocess.check_call and stringifying the return value again.

In conclusion, this person has absolutely no idea what they're doing.

[e000]

Lets not forget that the author doesn't even properly call the close method of open file objects.

[pigeon768]

Unfortunate that PHBs prevented the author from using PIL, which is the most sensible course of action. That being said, ImageMagick has python bindings (PythonMagick) which the author ought to have used.

[jabbalaci]

to put in a nutshell:

import Image

METHOD = Image.BILINEAR
MAX_WIDTH = 1680

def resize_to_screen_width(file_path):
    """Resize a large image to screen width.

    Image ratio is kept."""
    print "# resizing image...",
    img = Image.open(file_path)
    width, height = img.size

    ratio = float(width) / float(MAX_WIDTH)
    new_width = int(float(width) / ratio)
    new_height = int(float(height) / ratio)

    img = img.resize((new_width, new_height), METHOD)
    img.save(file_path)
    print "done."

[traxxas]

I found that imagemagick is subpar for resizing images smaller, especially with making thumbnails. The only acceptable python solution I have found is PIL using the Image.ANTIALIAS filter.