First practical Python code, comments appreciated
planetthoughtful
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
Steve Holden
> 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/
Paul McGuire
news:mailman.2080.1134563...@python.org...
> 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
planetthoughtful
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
Steve Holden
[...]
> portability is an n-way street.
Steve Holden
[...]
> portability is an n-way street.
Kent Johnson
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