Help me improve my Python Skills, please

[Trebor A. Rude]

I just started picking up Python a few days ago, and am rather
enjoying it.  To help me learn it, I converted one of my old TCL
scripts into Python, which I've attached below. However, this being my 
first serious python script, I'm pretty sure I'm doing some things
less efficiently than necessary, through lack of familiarity with the
library and language in general. So I'd like to enlist the group's
help in improving the code. Logic, style, better ways of doing things, 
whatever.  Thanks in advance.

A little explanation before the code.  The script recursively descends 
a directory structure, building a .jdb (Jukebox database) file for
each directory based on the mp3 tag info of the mp3s in that
directory and all its subdirectories. It also repairs damaged comment
fields caused by a slightly buggy encoder program.

#!/usr/bin/env python

# Does this look strange? My basic philosophy on
# using "import" vs. "from <foo> import <bar>" is
# based on my conception of how easy it might be
# to misinterpret the meaning of the function. Also,
# if I need a bunch of names from a package, I'll
# tend toward "import".

import os
import re
import select

from os.path import join, basename, isdir, splitext
from glob import glob
from fnmatch import fnmatch
from operator import concat
from commands import getoutput

# Only reason this is a function is because it gets called recursively.

def Make_Playlist ():
    pwd = os.getcwd()
    filename = join(pwd, basename(pwd) + '.jbd')

    try:
        listfile = open(filename, 'w')
    except IOError:
        sys.exit("Cannot open " + filename + " for write: " + IOError)

    # Since sort() returns None, need to go through these extra
    # steps instead of just putting all on the for line.
    
    files = glob(join(pwd, '*'))
    files.sort()
    
    for file in files:
        if isdir(file):   # Recursively descend directory.
            os.chdir(file)
            Make_Playlist()
            os.chdir(pwd)

            # Concatente the just-created .jbd onto the current one.
            
            infile = open(join(file, basename(file) + '.jbd'))
            listfile.writelines(infile.readlines())
            infile.close()
        elif fnmatch(file, '*.mp3'):
            newfilename = re.sub(' ', '_', basename(file))
            if (newfilename != basename(file)):
                os.rename(file, newfile)
                file = join(pwd, newfile)

            # Don't really need this since getting a better encoder,
            # but still might want to make it optional.
            
            #artist = re.sub('_', ' ', basename(pwd))
            #title = re.sub('_', ' ', splitext(basename(file))[0])
            #os.system("mp3info -t " + title + " -a " + artist \
            #+ " -g 1 " + file)

            # Yes, I really do have filenames which use all the
            # "bad for the shell" characters below.
            
            escaped_filename = re.sub('(["\'()& ])', r'\\\1', file)
            
            # Deleted the actual format because it was too long.
            # If anyone's interested, just ask.

            mp3info = getoutput('mp3info -f "..." + escaped_filename)

            # The encoder program I use is slightly buggy and tends to 
            # leave spaces followed by one random control character at 
            # the end of the comment field of the mp3 tag. Generally,
            # this isn't a problem, but it IS if that control
            # character is ^J, so I fix it here.

            # Notice the (\B|\b) construct.  What I really wanted to
            # do was make the whole of \1 optional, but then it causes 
            # an error if \1 doesn't match, which is bad because the
            # point is to delete the stuff after it.  Is there a
            # better way to do this?

            fixedinfo = re.sub(\
            r'\|((\B|\b)([A-Za-z \'/.]*[A-Za-z])?) +[\1-\26]\|', \
            r'|\1|', \
            reduce(concat, mp3info))

            # Hmm, come to think of it, I might want to repair the
            # actual mp3 file here.  I'll have to look into the
            # "Matcher" objects. (Would that be the right way to save
            # \1 from above?)

            listfile.write(fixedinfo)
            listfile.write('\n')
            
    listfile.close()

# os.path.walk() might simplfy things mightily, but would require some
# information to be saved outside the function to keep track of
# just-constructed .jbd files for inclusion in the current one.

Make_Playlist()



-- 
Trebor A. Rude
trebor at bwn.net
Registered Linux User #89308
http://counter.li.org/