||[Aug. 15th, 2007|02:41 am]
Please criticize my first Python program.
I really should read a book. What's everybody's favorites?
You will do fine (actually, you do do fine) with the stuff on the Python's site. Being a partisan, I will however recommend the "Python's cookbook", for which a friend of mine wrote a chapter or two.
The only thing I can see to criticize at first glance is that it might be worth checking whether the exception is one you expect before ignoring it.
Also, if this is Python 2.5, look at using "with" to simplify your use of files:http://docs.python.org/whatsnew/pep-343.html
Dive into Python (http://www.diveintopython.org/) by Mark Pilgrim is a must read!
I also recommend checking out his blog, good stuff.
2007-08-15 10:54 am (UTC)
file is a builtin, like open. You probably shouldn't use it as a variable for the same reason you're not using list as a variable.
You can iterate over dicts with for k, v in data.items(). You could del data[k] after the unlink.
2007-08-15 10:55 am (UTC)
also, try using
pylint (should be in Debian).
If you're iterating through a collection and deleting stuff, you ran into the obvious problem. The pythonic solution to that is something like:
for k in data.keys():
That way you don't have to fiddle with todel.
for k in data[:]:
is another way I saw recently.
Back to the program, the biggest problem I have with mailman (although it's probably true of most mailing list managers) is that there's not much of a public API apart from a few command-line programs. Of course, I just went at looked at the docs, and bin/withlist provides exactly what I want.
2007-08-15 11:12 am (UTC)
This is funny. Mostly because it's true:
This has nice coverage of Python's functional support:
Some interesting content in these videos:
list comprehensions are (trivially) cooler than the filter builtin
lists = filter(os.path.isdir, os.listdir("."))
lists = [d for d in os.listdir(".") if os.path.isdir(d)]
might want to throw your top-level code in a def main() and add a if __name__ == '__main__': main() to the top level. makes for easier organization (and makes sure that if your code is imported, it isn't run automatically, not that it matters here in this instance)
you can do type(k) != int, because int is a type (along with str, float, etc). saves an import and some typing
other than that, not bad for a first python program. maybe you could critique my first perl program when i get around to writing it :)
2007-08-15 12:04 pm (UTC)
If you want some advice regarding the "pythonic" style you can take a look here:
Regarding your first program, I suggest you use isinstance instead of checking directly the type (see pep-8 also).
>>> type(1) != types.IntType
>>> not isinstance(1, int)
ciphergoth is right regarding the with statement and, as ghewgill said, you can improve the way you're working with the data dictionary. You can modify it during the iteration if you use the keys() or items() methods (just be sure you're not using one of the iter* equivalents). I would probably use data.items() instead of data.keys() so you have direct access to the key and it's value.
Favorites books? Dive Into Python it's free and great (only lack is that it covers python 2.2 IIRC), Learning Python and Python in a Nutshell have been updated to 2.5 I think. The python tutorial is also a great source.
Great to see you stepping on python. :-)
2007-08-15 12:13 pm (UTC)
The Laughing Jesus by Timothy Freke!
Dive Into Python
is generally recommended for people who know how to program and want to learn Python without having to read about what a variable or a string is.
Quick things I noticed:
Docstrings are the convention for whole-program and whole-function notes. This means you want:
Remove moderation items from GNU Mailman's pending queue.
Also, as kinda noted below, if you do:
if __name__ == '__main__':
for lst in lists:
print "Cleaning list %s ..." % lst
you'll be able to use the program as a module ( without executing the main stuff automatically.
Generally you want to use instanceof instead of comparing types directly. I'm pretty sure this applies to built-in types as well, as it's a problem most commonly run into with string types (where ascii strings have one type, and unicode strings have a "unicode" type, but they both inherit from basestring). It's like using ->isa instead of "blessed($ref) eq".
The filter could alternately have been written as a list comprehension, but it's actually probably clearer as a filter (which is kind of unusual). If you're curious, I think the list comprehension would have been:
[x for x in os.listdir(".") if os.path.isdir(x)]