Processing eBay category list for database import

Question

Although I've been programming a fair while I'm learning Python and would like to know how I can make my code more "Pythonic".

I'm have a CSV file of eBay categories, containing the name and numeric code which I want to import in to my database. Unfortunately the file does not include a 'parent' category ID for each row, instead it uses the ordinal position of the name in row to indicate the nesting level:

L1,L2,L3,L4,L5,L6,Category ID
Antiques,,,,,,20081
,Antique Clocks,,,,,13851
,,Bracket Clocks,,,,100904
,,,Pre-1900,,,66840
,,,Post-1900,,,96762
,,Cuckoo Clocks,,,,13850
,,Longcase/ Grandfather Clocks,,,,100905
,,,Pre-1900,,,60249
,,,Post-1900,,,96763
,Antique Furniture,,,,,20091
,,Armoires/ Wardrobes,,,,98436

I've written a Python script to process the file, which is working correclty and generating the output I need, but I would like to know how it can be improved.

The script below is slightly changed from the one I am using, with simplified output (making it less verbose) but if anyone is interested in the other version which generates a JSON file for importing in to a Django site database, let me know.

I wanted to process the file line-by-line and avoid loading all the data in to memory, in case the file being processed is huge. So it uses a stack to record the parent category ID, other than that very little information is stored.

import csv


class Stack:
    def __init__(self):
        self.items = []

    def push(self, o):
        self.items.append(o)

    def pop(self, n=1):
        while n > 1:
            self.items.pop()
            n -= 1
        return self.items.pop()

    def peek(self, n=1):
        return self.items[-n] if len(self.items) >= n else 'null'


def extract_info(row):
    """
    :param row: data extracted from the csv file
    :return: level (number), the name of the category (string), category ID (number)

    Steps through the csv row until the first populated field (category name)
    and then returns the nesting level, category name and ID (which is always in
    the last field of the row.
    """
    level = 0
    for field in row:
        if field:
            return level, field, row[-1]
        level += 1


id_stack = Stack()

with open('UK_New_Structure_(Oct2015).csv', 'rb') as csvfile:
    rdr = csv.reader(csvfile, delimiter=',', quotechar='"')
    # Skip three header rows
    next(rdr)
    next(rdr)
    next(rdr)
    first_row = next(rdr)
    parent_id = 'null'  # top level categories do not have a parent

    level, name, cat_id = extract_info(first_row)
    print level, name, cat_id, parent_id
    id_stack.push(cat_id)  # Save first parent ID
    last_level = -1

    for row in rdr:
        level, name, cat_id = extract_info(row)

        if level == last_level:  # remove the newly added parent id, last item was not a parent
            id_stack.pop()
            parent_id = id_stack.peek()
            id_stack.push(cat_id)
        elif level < last_level:
            id_stack.pop(last_level - level + 1)
            parent_id = id_stack.peek()
            id_stack.push(cat_id)
            last_level = level
        elif level > last_level:
            parent_id = id_stack.peek()
            id_stack.push(cat_id)
            last_level = level

        print level, '..' * level, name, cat_id, parent_id

I am using Python 2.7.10, but if there are better ways to do this in Python 3, then I'm interested in those as well.

Answer 1

I don't have many comments regarding your style, as it seems to follow PEP8 (to my knowledge), so lets review the actual code:

• Implementation of Stack – A straight forward implementation without to much fuzz. You don't have any error checking related to popping something of an empty stack, which might give you a problem at some point
• Implementation of extract_info – No error handling if faulty input, which could let it return nothing at all if nothing matches on the if field: line
• rdr is not a good name – It is not clear what next(rdr) actually does, try finding a better name here
• Simplify if structure – You repeat statements within the if structure, and these could be moved outside of the if structure, which would help readability. In addition you pop/push too many times. Try the following:

if level <= last_level:
    id_stack.pop(last_level - level + 1)

parent_id = id_stack.peek()

id_stack.push(cat_id)
last_level = level

• Change to newer print – The currently recommended standard for printing seems to be of the form: print('{} {} {} {} {}'.format(level, '..'*level, name, cat_id, parent_id)). In your case it complicates stuff, but in the long run you can pretty print much better using the newer form and variation thereover

Edit: Original code had a little flaw regarding the parent_id. The now corrected code gives the same output when using the provided CSV file as the output from original code.

Answer 2

You can get the index of your iteration using enumerate instead of manually counting:

for i, field in enumerate(row):
    if field:
        return i, field, row[-1]

Repeating the exact same command is silly, just use a for loop instead. If you ever need to change the amount of lines you skip, this makes it far easier and it's easier to count.

# Skip three header rows
for _ in range(3):
    next(rdr)

Note I'm using _ to indicate that the value is throwaway and I really just want to loop three times.

Try to avoid long inline comments like this:

if level == last_level:  # remove the newly added parent id, last item was not a parent

It's hard to read and harder to edit, splitting over two lines is better in these cases.

if level == last_level:
    # Remove the newly added parent id, last item was not a parent

In your if and elif conditions your second elif should be an else so it's obvious that every case will fit into one of these conditions.

    if level == last_level: 
        id_stack.pop()
        parent_id = id_stack.peek()
        id_stack.push(cat_id)
    elif level < last_level:
        id_stack.pop(last_level - level + 1)
        parent_id = id_stack.peek()
        id_stack.push(cat_id)
        last_level = level
    else:
        parent_id = id_stack.peek()
        id_stack.push(cat_id)
        last_level = level