[CentralOH] Improving Zipfile Code

[Eric Floehr]

Jim,

The biggest thing that smells to me is the magic number '2'. I assume
that's because you are assuming a header row since you are opening a
DictReader, but still want to get the line number of the csv row.
 Thankfully, csv readers expose the 'line_num' property, so you can get the
line number of the file directly.

Also, while not 'smelly', the anonymous function (the lambda) can be
cleaned up by using the standard library module fnmatch, which is
specifically built for file matching.

You can also break up your combo opening the zip file and creating a
DictReader in the for loop, which doesn't do anything other than making it
look a little cleaner and makes it clearer at a glance what's going on.

Finally, since 'rU' is something you need to look up to understand, I put a
comment so it makes it clear what the 'rU' is for.

With those revisions, here is the updated code:

import csv
import fnmatch
from zipfile import ZipFile

def foo(zipfilename):
    z = ZipFile(zipfilename)
    for filename in fnmatch.filter(z.namelist(), '*.xyz'):
        csvfile = z.open(filename, 'rU')  # Open with universal newline
support
        csvreader = csv.DictReader(csvfile)
        for row in csvreader:
            print 'line_number:', csvreader.line_num, 'row:', row


-Eric


On Sun, Oct 21, 2012 at 10:46 PM, <jep200404 at columbus.rr.com> wrote:

> The following code smells. How would you improve it?
>
> import csv
> from zipfile import ZipFile
>
> def foo(filename):
>     z = ZipFile(filename)
>     for f in filter((lambda s: s.endswith('.xyz')), z.namelist()):
>         for line_number, row in enumerate(csv.DictReader(z.open(f, 'rU')),
> 2):
>             print 'line_number:', line_number, 'row:', row
>
> _______________________________________________
> CentralOH mailing list
> CentralOH at python.org
> http://mail.python.org/mailman/listinfo/centraloh
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/centraloh/attachments/20121022/571f1b52/attachment.html>