views:

124

answers:

1

heya,

I have a Excel CSV files with employee records in them. Something like this:

mail,first_name,surname,employee_id,manager_id,telephone_number
[email protected],john,smith,503422,503423,+65(2)3423-2433
[email protected],george,brown,503097,503098,+65(2)3423-9782
....

I'm using DictReader to put this into a nested dictionary:

import csv
gd_extract = csv.DictReader(open('filename 20100331 original.csv'), dialect='excel')
employees = dict([(row['employee_id'], row) for row in gp_extract])

Is the above the proper way to do it - it does work, but is it the Right Way? Something more efficient? Also, the funny thing is, in IDLE, if I try to print out "employees" at the shell, it seems to cause IDLE to crash (there's approximately 1051 rows).

2. Remove employee_id from inner dict

The second issue issue, I'm putting it into a dictionary indexed by employee_id, with the value as a nested dictionary of all the values - however, employee_id is also a key:value inside the nested dictionary, which is a bit redundant? Is there any way to exclude it from the inner dictionary?

3. Manipulate data in comprehension

Thirdly, we need do some manipulations to the imported data - for example, all the phone numbers are in the wrong format, so we need to do some regex there. Also, we need to convert manager_id to an actual manager's name, and their email address. Most managers are in the same file, while others are in an external_contractors CSV, which is similar but not quite the same format - I can import that to a separate dict though.

Are these two items things that can be done within the single list comprehension, or should I use a for loop? Or does multiple comprehensions work? (sample code would be really awesome here). Or is there a smarter way in Python do it?

Cheers, Victor

+2  A: 

Your first part has one simple issue (which might not even be an issue). You don't handle key collisions at all (unless you intend to simply overwrite).

>>> dict([('a', 'b'), ('a', 'c')])
{'a': 'c'}

If you're guaranteed that employee_id is unique, there isn't an issue though.

2) Sure you can exclude it, but no real harm done. Actually, especially in python, if employee_id is a string or int (or some other primitive), the inner dict's reference and the key actually reference the same thing. They both point to the same spot in memory. The only duplication is in the reference (which isn't that big). If you're worried about memory consumption, you probably don't have to.

3) Don't try to do too much in one list comprehension. Just use a for loop after the first list comprehension.

To sum it all up, it sounds like you're really worried about the performance of iterating over the loop twice. Don't worry about performance initially. Performance problems come from algorithm problems, not specific language constructs like for loops vs list comprehensions.

If you're familiar with Big O notation, the list comprehension and for loop after (if you decide to do that) both have a Big O of O(n). Add them together and you get O(2n), but as we know from Big O notation, we can simplify that to O(n). I've over simplified a lot here, but the point is, you really don't need to worry.

If there are performance concerns, raise them after you written the code and prove it to yourself with a code profiler.

response to comments

As for your #2 reply, python really doesn't have a lot of mechanisms for making one liners cute and extra snazzy. It's meant to force you into simply writing the code out vs sticking it all in one line. That being said, it's still possible to do quite a bit of work in one line. My suggestion is to not worry about how much code you can stick in one line. Python looks a lot more beautiful (IMO) when its written out, not jammed in one line.

As for your #1 reply, you could try something like this:

employees = {}
for row in gd_extract:
    if row['employee_id'] in employees:
        ... handle duplicates in employees dictionary ...
    else:
        employees[row['employee_id']] = row

As for your #3 reply, not sure what you're looking for and what about the telephone numbers you'd like to fix, but... this may give you a start:

import re
retelephone = re.compile(r'[-\(\)\s]') # remove dashes, open/close parens, and spaces
for empid, row in employees.iteritems():
    retelephone.sub('',row['telephone'])
xyld
heya,@xyld: Thanks for the detailed reply =).1. The employee_id is guaranteed to be unique, from what I've been told. However, just for reference, what's the recommended way of handling this? Exceptions? (Any sample code here would be awesome).2. Fair enough, if it's just some extra memory, then I suppose it's no biggie. I just thought there might be a clever way in Python to exclude it from the inner dict.
victorhooi
3. Hmm, yeah, you're right, premature optimisation is the root of all evil, and all that *grins*. And yeah, it's just linear, so I'm sure it'll be fine. I'm still trying to get my head around comprehensions though, is there a way to use one to do a regex replace on telephone_number, and the lookups for the manager?
victorhooi
@victorhooi you're too hung up on list comprehensions I think. Just use a for loop. List comprehensions should come naturally in the code to make it look cleaner, not just for **fun**. IMO
xyld
@xyld: Fair enough, yeah, I'm doing it with a for loop now =). Thanks for all your help.
victorhooi