Command line utility to convert image formats of photos

Question

Below is some code for a basic command line utility to convert image formats of photos. I'm wondering how any part of it might be written with better style, i.e. how it could be neater, clearer and generally "better".

import os
import argparse
from PIL import Image
def get_arguments():
 ''' Get command line arguments '''
 parser = argparse.ArgumentParser(
 description='A command line tool to convert images to different formats')
 parser.add_argument(
 'input', 
 help='full path to the input file or folder')
 parser.add_argument(
 'outext',
 choices=Image.registered_extensions().keys(),
 help='output extension for file')
 parser.add_argument(
 '-n', '--new-name',
 help='give the output file a new name')
 parser.add_argument(
 '-d', '--destination', 
 help='set new destination directory') 
 parser.add_argument(
 '-R', '--remove-input', 
 action='store_true',
 help='also remove the input file')
 args = parser.parse_args()
 if not (os.path.isfile(args.input) or os.path.isdir(args.input)):
 # easier to check file validiy here and keep infile as str,
 # rather than use type checking in parser.add_argument()
 raise parser.error('Unrecognised input type - input must be a file or folder')
 else:
 return args
def convert_image(infile, outext, outname=None, outdir=None, remove_infile=False):
 ''' Convert an image to a new format and optionally:
 * Change the filename
 * Put the file in a new directory
 * Remove the input file
 '''
 indir, inname = os.path.split(infile)
 inname = os.path.splitext(inname)[0]
 if outname is None:
 outname = inname
 if outdir is None:
 outdir = indir
 outfile = os.path.join(outdir, '{}{}'.format(outname, outext))
 if not os.path.isdir(outdir) and outdir != '':
 os.makedirs(outdir)
 Image.open(infile).save(outfile)
 if remove_infile:
 os.remove(infile)
def convert_images(im_dir, outext, outname=None, outdir=None, remove_infile=False):
 ''' run convert_image() for all files in a folder '''
 for item in os.listdir(im_dir):
 if os.path.splitext(item)[1] in Image.registered_extensions().keys():
 convert_image(
 os.path.join(im_dir, item),
 outext,
 outname=outname,
 outdir=outdir,
 remove_infile=remove_infile)
def main():
 args = get_arguments()
 if os.path.isdir(args.input):
 convert_images(
 args.input,
 args.outext,
 outname=args.new_name,
 outdir=args.destination,
 remove_infile=args.remove_input)
 elif os.path.splitext(args.input)[1] in Image.registered_extensions().keys():
 convert_image(
 args.input,
 args.outext,
 outname=args.new_name,
 outdir=args.destination,
 remove_infile=args.remove_input)
 else:
 raise ValueError('Unsupported file format')
if __name__ == '__main__':
 main()

Answer 1

Specific suggestions:

1. I generally find that ordering functions by their importance is helpful. Moving get_arguments just above or below main would allow readers to get into the functionality immediately. Others prefer to write methods in call order, but this feels a bit arbitrary (should main be first or last?) and not always achievable, because with branching it's possible to have a cycle of callers.
2. *nix tools usually support any number of paths as their last arguments, operating on each of them. This would certainly be useful here, and means you could even avoid the whole if/else in main because all inputs should be file paths.
3. argparse has a file type which you can use.
4. Don't rely on file extensions to correspond to file formats. I would instead rely on PIL to detect the file type and to throw an exception if it doesn't know it.
5. Abbreviations like inname make the code harder to read. One way to avoid the urge to shorten everything is to use an IDE, because it'll help you auto-complete everything which is in scope.
6. In the end this script is a small convenience wrapper around PIL. This is one of those rare occasions when I would suggest replacing it with a small shell script.

General suggestions:

1. black can automatically format your code to be more idiomatic.
2. isort can group and sort your imports automatically.

3. flake8 with a strict complexity limit will give you more hints to write idiomatic Python:

   [flake8]
   max-complexity = 4
   ignore = W503,E203
   

4. I would then recommend adding type hints everywhere and validating them using a strict mypy configuration:

   [mypy]
   check_untyped_defs = true
   disallow_untyped_defs = true
   ignore_missing_imports = true
   no_implicit_optional = true
   warn_redundant_casts = true
   warn_return_any = true
   warn_unused_ignores = true
   

• Matt: Re point 6, I understand that this could be written as a bash (or similar) script, but is is there a tangible advantage of doing so, or is this just something one might do as there is no need to use Python?
• l0b0: Well, a Bash script doing basically the same thing might be 10 lines. It won't win a beauty contest, but this isn't the sort of software which would be distributed to lots of people, so there's a trade-off of "getting stuff done" vs. "doing it right."
• Cris Luengo: You don’t even need a Bash script. Just download ImageMagick if you don’t have it already, it has a convert program that’s really easy to use. @Robert