2
\$\begingroup\$

I have written code that reads in a CSV file and creates a tuple from all the items in a group. The group ID is in column 1 of the table and the item name is in column 2. The actual datafile is ~500M rows.

Is there any way to make this code more efficient?

Input file:

"CustID"|"Event"
1|Alpha
1|Beta
1|AlphaWord
1|Delta
2|Beta
2|Charlie
2|CharlieSay

Code:

def sequencer(myfile):
    import csv
    counter = 1
    seq = []
    sequence = []
    with open(myfile, 'rb') as csvfile:
        fileread = csv.reader(csvfile, delimiter='|', quotechar='"')
        next(fileread) ## skip header
        for row in fileread:
            #if counter == 5:
            #    break
            if 'word' in row[1] or 'say' in row[1]: ##if event has either word or say anywhere in the text then ignore (eg: even ignore adword or afdjklSAYwer)
                continue
            if int(row[0]) == counter:
                seq.extend([row[1]])
            else:
                sequence.append(seq)
                seq = [row[1]]
                counter = counter+1
        sequence.append(seq)
    return sequence

Output:

An array which is a list of lists, where each list is the sequence of events in the order of the text file for each customer ID.

\$\endgroup\$
3
\$\begingroup\$

The problem is that a list made from the rows of a file with ~500M rows will have millions of entries, and that’s going to really hamper Python. As long as you’re using a list, I don’t think there’s any magic you can do here to magically fix that problem.

So you should ask: do I need a list? You may be able to make to do with a generator, which is much faster and more memory efficient (because it only computes one element at a time, not precomputing everything).

For a generator, I’m assuming that the customer IDs are monotonically increasing. If not, you really do have to go through the entire file to be sure you’ve got everything for each group. But your existing code seems to assume that, so I’ll assume I can as well.


Here’s a slight reworking of your code to use generators:

import csv

def sequencer(myfile):
    """
    Reads customer IDs and events from a CSV file.  Yields events for
    successive groups.
    """
    with open(myfile, 'rb') as csvfile:
        reader = csv.reader(csvfile, delimiter='|', quotechar='"')
        next(reader) ## skip header

        current_cust_id = 1
        seq = []
        for row in reader:
            cust_id, event = row

            if ('word' in event) or ('say' in event):
                continue

            while int(cust_id) != current_cust_id:
                yield seq
                seq = []
                current_cust_id += 1
            seq.append(event)

        yield seq

I’ve done away with the massive list (sequence), and we just yield the list of per-customer events whenever the customer ID changes.


A few other comments on your code:

  • Put your imports at the top of your file; don’t hide them inside a function.
  • There isn’t much in the way of documentation. Your function should at least have a docstring, and some more comments would be nice. For example, tell me why we’re skipping lines with “Say” or “Word”. I can read that the code is doing it, but I can’t read your mind to find out why.
  • You were assuming that customer IDs increase by 1 every time, and that there’s never going to be a gap. What happens if you run your code against:

    "CustID"|"Event"
    1|Alpha
    1|Beta
    1|AlphaWord
    1|Delta
    2|Beta
    2|Charlie
    2|CharlieSay
    5|Echo
    5|Delta
    

    Maybe you know that will be the case; I wasn’t sure, so my code is defensive against this possibility.

  • Rather than doing seq.extend([row[1]]), it’s much cleaner to do seq.append(row[1]). It saves you creating an additional list.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks. I have updated my complete code and submitted here for overall code review \$\endgroup\$ – ADatoo Oct 22 '15 at 23:08
2
\$\begingroup\$

You could use itertools.groupby for grouping:

import csv
import itertools
import operator

def sequencer(myfile):
    sequence = []
    with open(myfile, 'rb') as csvfile:
        fileread = csv.reader(csvfile, delimiter='|', quotechar='"')
        next(fileread) ## skip header
        for cust_id, group in itertools.groupby(fileread, operator.itemgetter(0)):
            events = [event for _, event in group 
                      if 'word' not in event and 'say' not in event]
            if events:
                sequence.append(events)
    return sequence
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks, but I could not get this code to work. The line events = [event for _, event in group if 'word' not in event and 'say' not in event] said that I cannot do events on a literal \$\endgroup\$ – ADatoo Oct 20 '15 at 10:54
  • \$\begingroup\$ @ADatoo Hmm, what version of Python do you have? What is the exact error message? \$\endgroup\$ – Janne Karila Oct 20 '15 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.