Automating the installation and configuration of Vim

Question

I am writing a script to automate the install and configuration of vim, zsh, ohmyzsh and tmux, on a Linux/Mac. I'd like to perform all operations in Python, rather than using subprocess to streamline some of the operations (e.g. git clone).

I would also like to use this script as an example to myself of good Python coding practice for file system operations. Code clarity, reuse and pythonic style are my primary objectives. Before I write the remainder of the script I'd like some input on the methods I've chosen for low level operations, but all areas of the script are open for scrutiny.

from urllib.request import urlretrieve
from urllib.error import URLError
from zipfile import ZipFile
import sys
import os
import errno
import shutil


vim = ".vimtest"
vim_directories = [vim, (vim + "/bundle"), (vim + "/autoload"), (vim + "/colors")]
vim_files = [".vimrctest", vim + "/vimrc"]
flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY


def vim_install():

    # Create directories and files

    for sub in vim_directories:
        try:
            os.mkdir(os.path.expanduser('~/' + sub))
            print("mkkdir " + os.path.expanduser('~/' + sub))
        except OSError as e:
            if e.errno == errno.EEXIST:
                print("Directory " + vim + sub + " already exists.")
                sys.exit(1)
            else:
                raise

    try:
        file_handle = os.open(os.path.expanduser('~/' + vim_files[1]), flags)
    except OSError as e:
        if e.errno == errno.EEXIST:
            print("File " + vim_files[1] + " already exists")
            sys.exit(1)
        else:
            raise
    with os.fdopen(file_handle, 'w') as file_obj:
        file_obj.write("")
    print("Created file " + os.path.expanduser('~/') + vim_files[1])

    try:
        os.symlink(os.path.expanduser('~/') + vim_files[1], 
        os.path.expanduser('~/') + vim_files[0])
        print("ln -s " + os.path.expanduser('~/') + vim_files[0] + " -> " +
        os.path.expanduser('~/') + vim_files[1])

    except OSError:
        print("Failed to make symbolic link.")
        print("ln -s " + os.path.expanduser('~/') + 
        vim_files[0] + " -> " + os.path.expanduser('~/') +
        vim_files[1])
        sys.exit(1)

    # Download Pathogen, NerdTree, unpack

    try:
        urlretrieve("https://tpo.pe/pathogen.vim",
                        os.path.expanduser('~/' + vim + "/autoload/pathogen.vim"))
        print("Downloaded Pathogen")

        urlretrieve("https://github.com/scrooloose/nerdtree/archive/master.zip",
                        os.path.expanduser('~/' + vim + "/bundle/master.zip"))
        print("Downloaded NerdTree")

    except URLError as e:
        if hasattr(e, 'reason'):
            print('Scrip failed to reach a server.')
            print('Reason: ', e.reason)
        elif hasattr(e, 'code'):
            print('The server couldn\'t fulfill the request.')
            print('Error code: ', e.code)
       else:
            raise

    try:
        zip_ref = ZipFile(os.path.expanduser('~/') + vim + 
        "/bundle/master.zip", 'r')
        zip_ref.extractall(os.path.expanduser('~/') + vim + "/bundle/")
        zip_ref.close()
        print("Unziped NerdTree")

    except OSError as e:
        print("Could not unzip: {0}".format(e))

    try:
        os.rename(os.path.expanduser('~/' + vim + "/bundle/nerdtree-master"),
        os.path.expanduser('~/' + vim + "/bundle/nerdtree"))
        print("Renamed " + os.path.expanduser('~/') + vim + 
        "/bundle/nerdtree-master \n"
          + os.path.expanduser('~/') + vim + "/bundle/nerdtree")

    except OSError as e:
        print("OS error {0}".format(e))

Answer 1

• vim, vim_directories, vim_files and flags seem to be constants, so you should apply the naming conventions on them by capitalizing their names.

Reviewing the very first part of your function code

About the serious things, I would talk for the moment only about the very first part of your code, later I may come back (no promise though):

• Within the try block, you should test only the code that is actually prone to raising an exception, nothing else. If you apply this rule, you will end up by coding short try blocks.
• What may raise an exception here is rather the attempt to create a directory: os.mkdir(os.path.expanduser('~/' + sub)), the print() would not execute anyway if the attempt fails. If the directory creation succeeds, the else block should take care of the printing task. The try block should be concise and clean
• I do not see the necessity of sys.exit(1) here. To be honest, in most cases it is useless, and in your typical case, I do not see how it could be useful, and you already print a message about execution failure.

• As a reader of your code, I was curious to know why are you particularly interested errno.EEXIST but not other error codes? I was expecting that you are too meticulous and you decided to track down all the symbols of errno module, but I was mistaken. I think it is fair enough to write this instead:

  except OSError as e:
       print('Error code: {}'.format(e.errno))
       print('Error message: {}'.format(e.strerror))
  

• Besides, note that this code:

  e.errno == errno.EEXIST:
      print('...')
  

  is the same as:

  e.errno == 17:
      print('...')
  

  What I mean here, do you really want to include such fine grain details in a function which is rather more generic/high level?

• If for some mysterious reason of your own, you find errno.EEXIST too sexy and attractive, I would still opt for a less brain nagging syntax: FileExistsError instead of errno.EEXIST (note that the later corresponds exactly to FileExistsError but, as you can see, it is more readable)

• Given the elements mentioned so far, I would write the very first part of your code as follows:

  for sub in vim_directories:
      try:
          os.mkdir(os.path.expanduser('~/' + sub)) 
      except FileExistsError:
          print("Directory " + vim + sub + " already exists.")         
      except OSError as e:
          print('Error code: {}'.format(e.errno))
          print('Error message: {}'.format(e.strerror))
      else:
          print("mkkdir " + os.path.expanduser('~/' + sub))