First practical Python code, comments appreciated

Discussion in 'Python' started by planetthoughtful, Dec 14, 2005.

  1. Hi All,

    I've written my first piece of practical Python code (included below),
    and would appreciate some comments. My situation was that I had a
    directory with a number of subdirectories that contained one or more
    zip files in each. Many of the zipfiles had the same filename (which is
    why they had previously been stored in separate directories). I wanted
    to bring all of the zip files (several hundrd in total) down to the
    common parent directory and obviously rename them in the process by
    appending a unique string to each filename.

    The code below did the job admirably, but I don't imagine it is
    anywhere near as efficiently written as it could and should be. Note:
    I'm aware there was no need, for example, to wrap the "os.walk(path)"
    statement in a def -- in a couple of places I added extra steps just to
    help familiarize myself with Python syntax.

    I'd very much like to see how experienced Python coders would have
    achieved the same task, if any of you can spare a couple of minutes to
    share some knowledge with me.

    Many thanks and much warmth,

    planetthoughtful

    import os
    fcount = 0
    path = 'X:\zipfiles'
    def listFiles(path):
    mylist = os.walk(path)
    return mylist

    filelist = listFiles(path)
    for s in filelist:
    if len(s[2]) > 0:
    for f in s[2]:
    pad = str(fcount)
    if len(pad) == 1:
    pad = "00" + pad
    elif len(pad) == 2:
    pad = "0" + pad

    (fname, fext) = os.path.splitext(f)
    oldname = f
    newname = fname + "_" + pad + fext
    os.rename(s[0] + "\\" + oldname, path + "\\" + newname)
    fcount = fcount + 1
    planetthoughtful, Dec 14, 2005
    #1
    1. Advertising

  2. planetthoughtful

    Steve Holden Guest

    planetthoughtful wrote:
    > Hi All,
    >
    > I've written my first piece of practical Python code (included below),
    > and would appreciate some comments. My situation was that I had a
    > directory with a number of subdirectories that contained one or more
    > zip files in each. Many of the zipfiles had the same filename (which is
    > why they had previously been stored in separate directories). I wanted
    > to bring all of the zip files (several hundrd in total) down to the
    > common parent directory and obviously rename them in the process by
    > appending a unique string to each filename.
    >
    > The code below did the job admirably, but I don't imagine it is
    > anywhere near as efficiently written as it could and should be. Note:
    > I'm aware there was no need, for example, to wrap the "os.walk(path)"
    > statement in a def -- in a couple of places I added extra steps just to
    > help familiarize myself with Python syntax.
    >
    > I'd very much like to see how experienced Python coders would have
    > achieved the same task, if any of you can spare a couple of minutes to
    > share some knowledge with me.
    >
    > Many thanks and much warmth,
    >
    > planetthoughtful


    Brave of you. Please note I haven't actually tested the exact format of
    these suggestions, so I made have made stupid typos or syntax errors.
    >
    > import os
    > fcount = 0
    > path = 'X:\zipfiles'
    > def listFiles(path):
    > mylist = os.walk(path)
    > return mylist
    >
    > filelist = listFiles(path)
    > for s in filelist:
    > if len(s[2]) > 0:


    You don't really need this "if" - with an empty s the "for" loop on the
    next line will simply execute its body zero times, giving the effect you
    appear to want without the extra level of logic.

    > for f in s[2]:
    > pad = str(fcount)
    > if len(pad) == 1:
    > pad = "00" + pad
    > elif len(pad) == 2:
    > pad = "0" + pad
    >

    Here you could take advantage of the string formatting "%" operator and
    instead of the "if" statement just say

    pad = "%03d" % fcount

    >>> ["%03d" % x for x in (1, 3, 10, 30, 100, 300)]

    ['001', '003', '010', '030', '100', '300']

    > (fname, fext) = os.path.splitext(f)
    > oldname = f


    There isn't really any advantage to this assignment, though I admit it
    does show what you are doing a little better. So why not just use

    for oldname in s[2]:

    to control the loop, and then replace the two statements above with

    (fname, fext) = os.path.splitext(oldname)

    Note that assignments of one plain name to another are always fast
    operations in Pythin, though, so this isn't criminal behaviour - it just
    clutters your logic a little having essentially two names for the same
    thing.

    > newname = fname + "_" + pad + fext
    > os.rename(s[0] + "\\" + oldname, path + "\\" + newname)


    That form is non-portable. You might argue "I'm never going to run this
    program on anything other than Windows", and indeed for throwaway
    programs it's often easier to write something non-portable. It's
    surprising, though, how often you end up *not* throwing away such
    programs, so it can help to write portably from the start. I'd have used

    newname = os.path.join(path,
    "%s_%s.%s" % (fname, pad, fext))
    os.rename(os.path.join(s[0], oldname), newname)

    > fcount = fcount + 1
    >


    Just a few pointers to make the script simpler, but your program is a
    very creditable first effort. Let us know what mistakes I made!

    regards
    Steve
    --
    Steve Holden +44 150 684 7255 +1 800 494 3119
    Holden Web LLC www.holdenweb.com
    PyCon TX 2006 www.python.org/pycon/
    Steve Holden, Dec 14, 2005
    #2
    1. Advertising

  3. planetthoughtful

    Paul McGuire Guest

    "Steve Holden" <> wrote in message
    news:...
    > That form is non-portable. You might argue "I'm never going to run this
    > program on anything other than Windows", and indeed for throwaway
    > programs it's often easier to write something non-portable. It's
    > surprising, though, how often you end up *not* throwing away such
    > programs, so it can help to write portably from the start. I'd have used
    >
    > newname = os.path.join(path,
    > "%s_%s.%s" % (fname, pad, fext))
    > os.rename(os.path.join(s[0], oldname), newname)
    >
    > > fcount = fcount + 1
    > >

    >
    > Just a few pointers to make the script simpler, but your program is a
    > very creditable first effort. Let us know what mistakes I made!
    >


    Just to chime in, and say that Steve's comments on portable programming have
    to do with more than just your code running on other machines. Developing
    habits such as portable programming helps you to do these kinds of things
    naturally, rather than to have to make a special effort if/when the issue
    does come up. Meanwhile, your portfolio of developed code reflects a
    broader thinking span on your part, beyond just "let me whip together
    something quick for the problem at hand." When presenting your work, at a
    conference or a job interview, it always helps to convey that you can see
    the bigger picture.

    Also, developing portable coding habits makes it easier for *you* to
    assimilate code that others may have written for other platforms -
    portability is an n-way street. If you download some code fragment from
    SourceForge, or from a tutorial, or a PyCon presentation, you will be less
    likely to scratch your head over the purpose of os.join if you are in the
    habit of using it already.

    Otherwise, your code (with Steve's comments) is good, and it looks like it
    does the job. But I would also look over some tutorials, or examples of
    existing code - Python comes with a huge set of sample code in the provided
    libraries, and if you simply "self-teach" yourself, you can develop some bad
    habits, and remain ignorant of some nice features and best practices.

    Look at the use of native data structures (tuples, lists, and dicts), the
    use of classes and class hierarchies, the uses of the module library, and
    some of the relatively recent idioms of generators, list
    comprehensions/generator expressions. Look at the layout of the code to see
    how the author broke the problem into manageable pieces. Look for some of
    the modules that crop up over and over again - not just sys, os, and math,
    which are analogous to the C RTL, but also itertools, which I think is more
    in the category of useful magic, like the C++ STL's <algorithm> module.

    Look for object design concepts, such as containment and delegation, and ask
    yourself how well they fit the problem for the given script. If you are new
    to object-oriented coding as well, read some of the free online Python
    introductory texts, such as "Dive into Python" and "Think like a Computer
    Scientist in Python," to learn how some of the object design idioms
    translate to Python's type and language model.

    Python expertise comes in a series of plateaus, over time and with practice.
    It's a stimulating and rewarding journey, no matter which plateau you
    reach - welcome to Python!

    -- Paul
    Paul McGuire, Dec 14, 2005
    #3
  4. Thanks to both Steve and Paul!

    I actually come from a PHP background, and I'm learning Python, oddly
    enough, as a result of recently purchasing a USB Flash Drive, and
    through wanting to be able to carry a portable programming language on
    the drive so that I have the capability of developing / using useful
    code wherever I happen to be (I'm actually using Movable Python
    http://www.voidspace.org.uk/python/movpy/ - I hope it's not lacking in
    any fundamental way).

    To be honest, I could have achieved the same result more gracefully in
    PHP (and I know that will cause some eyebrows to arch over the
    suggestion that anything can be done gracefully in PHP), but that's
    simply because I'm very comfortable with its syntax and with writing
    economical code in it, not because it's actually well-suited to that
    type of task.

    I'm already working through Dive Into Python, which seems to be a good
    starting place. Thankfully, I'm comfortable with OO concepts, at least
    in how they are expressed in PHP5, so I'm not entirely lost in
    unfamiliar territory with those aspects of Python.

    My big learning curve will come, I suspect, when I move into creating
    GUI apps with Python and wxPython, since that's my end goal - to be
    able to carry several self-developed applications on my Flash Drive
    that aren't dependent on any resources not found on the Flash Drive.

    But, you have to learn to crawl before you can learn to code, so
    approaching basic tasks like the one in my first attempt at a practical
    application of Python are a good way to begin.

    Thanks to both for your comments and advice!

    Much warmth,

    planetthoughtful
    planetthoughtful, Dec 14, 2005
    #4
  5. planetthoughtful

    Steve Holden Guest

    Paul McGuire wrote:
    [...]
    > portability is an n-way street.

    +1 QOTW

    regards
    Steve
    --
    Steve Holden +44 150 684 7255 +1 800 494 3119
    Holden Web LLC www.holdenweb.com
    PyCon TX 2006 www.python.org/pycon/
    Steve Holden, Dec 14, 2005
    #5
  6. planetthoughtful

    Steve Holden Guest

    Paul McGuire wrote:
    [...]
    > portability is an n-way street.

    +1 QOTW

    regards
    Steve
    --
    Steve Holden +44 150 684 7255 +1 800 494 3119
    Holden Web LLC www.holdenweb.com
    PyCon TX 2006 www.python.org/pycon/
    Steve Holden, Dec 14, 2005
    #6
  7. planetthoughtful

    Kent Johnson Guest

    planetthoughtful wrote:
    > Hi All,
    >
    > I've written my first piece of practical Python code (included below),
    > and would appreciate some comments. My situation was that I had a
    > directory with a number of subdirectories that contained one or more
    > zip files in each. Many of the zipfiles had the same filename (which is
    > why they had previously been stored in separate directories). I wanted
    > to bring all of the zip files (several hundrd in total) down to the
    > common parent directory and obviously rename them in the process by
    > appending a unique string to each filename.
    >
    > The code below did the job admirably, but I don't imagine it is
    > anywhere near as efficiently written as it could and should be. Note:
    > I'm aware there was no need, for example, to wrap the "os.walk(path)"
    > statement in a def -- in a couple of places I added extra steps just to
    > help familiarize myself with Python syntax.
    >
    > I'd very much like to see how experienced Python coders would have
    > achieved the same task, if any of you can spare a couple of minutes to
    > share some knowledge with me.
    >
    > Many thanks and much warmth,
    >
    > planetthoughtful
    >
    > import os
    > fcount = 0
    > path = 'X:\zipfiles'
    > def listFiles(path):
    > mylist = os.walk(path)
    > return mylist


    Note that os.walk() doesn't return a list, it returns an iterable object (a generator).
    Your usage will work for either, but your names are misnomers.
    >
    > filelist = listFiles(path)
    > for s in filelist:


    I would write
    for dirpath, dirnames, filenames in filelist:

    which gives names to what you call s[0] and s[2]

    or just
    for dirpath, dirnames, filenames in os.walk(path):

    Kent

    > if len(s[2]) > 0:
    > for f in s[2]:
    > pad = str(fcount)
    > if len(pad) == 1:
    > pad = "00" + pad
    > elif len(pad) == 2:
    > pad = "0" + pad
    >
    > (fname, fext) = os.path.splitext(f)
    > oldname = f
    > newname = fname + "_" + pad + fext
    > os.rename(s[0] + "\\" + oldname, path + "\\" + newname)
    > fcount = fcount + 1
    >
    Kent Johnson, Dec 14, 2005
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Brad Jones

    Help or comments appreciated.

    Brad Jones, May 18, 2005, in forum: ASP .Net
    Replies:
    4
    Views:
    1,228
    Brad Jones \(not CodeGuru Guy!\)
    May 19, 2005
  2. Replies:
    3
    Views:
    404
    Roedy Green
    Sep 22, 2005
  3. Deryck
    Replies:
    7
    Views:
    464
  4. windandwaves
    Replies:
    18
    Views:
    688
    Spartanicus
    Mar 19, 2005
  5. Brian L. Troutwine
    Replies:
    5
    Views:
    279
    Nick Craig-Wood
    Jun 4, 2007
Loading...

Share This Page