Excel Mapping Module

Question

I wrote this bit of python intending it to become some module to replace excel mapping programs like MapForce. Right now it only does columns to columns. So the number of rows in always equals the number of rows out.

To declare a map, use a python defaultdict(list) object

from collections import defaultdict

demoCmd = defaultdict(list)
demoCmd = {
    'A': [mapOper.mapVal, 'Hello, World'],
    'B': [mapOper.mapSum, 0, 1, 2],
    'C': [mapOper.mapAss, 3, 4, 5],
    'D': [mapOper.mapProd, [mapOper.mapSum, 1, 0], 0, 1],
    'E': 5
    }

The keys are the destination columns name or index. Integers can be used as keys but they must be zero offset. Strings are converted to zero offset integers. The Lists are the functions mapping to that column. It's in LISP style [function arg1, arg2, etc], where integers are column indexes (zero offset) and strings are raw values. And you can nest the lists like in 'D'.

mapOper is just a file containing some commonly used functions. Some of them I don't think I need to include eg

def mapSum(*valueList):
    filter(None, valueList)
    return sum(valueList)

Anyways the main engine is this

def evalOpList(opList, fromSheet, row):
    if type(opList) is str:  # treat arg as a value
        return opList
    elif type(opList) is int:  # treat arg as index
        return fromSheet.cell(row, opList).value
    else:  # its another function
        args = list()
        itOpList = iter(opList)
        next(itOpList)
        args = [evalOpList(it, fromSheet, row) for it in itOpList]
        return opList[0](*args)


def interpColMap(mapCmd, fmSheet, toSheet, rTop=0, rBot=0, rToTop=0):
    if rBot < 0:
        rBot = fmSheet.nrows
    MapCmdConvert(mapCmd)
    assert rTop < rBot and rToTop >= 0
    for row in range(rTop, rBot):
        for key in list(mapCmd.keys()):
            toSheet.write(row, key, evalOpList(mapCmd[key], fmSheet, row))

And you call it like this

colMapper.interpColMap(demoCmd, fws, tws)

Where fws is an worksheet from xlrd to map from and tws is a workbook from xlwt to map to.

I have already tested it some and it works. It's just that coming from C++ this seems very odd style. Only two function make up my program and no classes. I also have the problem of having all ints be indexes and all strs be values. This means I can't do += 1 to a column without doing [int, '1']. And I don't even know if that would work.

It's just a very unique style and I need to know if it's too convoluted or not.

Answer 1

Generally camelCase is used for both locals and functions. Both PEP 8 and Google style guide recommend underscore lower_caps for both. camelCase isn't wrong in python, it's just sticking with global standards in more right.

Also some names could be more descriptive. rBot could be bottom_row. operator_list is more verbose than opList but it's name is now exactly what it is and there is no question of the programmers intention of the variable.

---

Initializing the demo command is done incorrectly despite it being functional code. It shows a lack of understanding of how variables work in python.

demoCmd = defaultdict(list)
demoCmd = {
    'A': [mapOper.mapVal, 'Hello, World'],
    'B': [mapOper.mapSum, 0, 1, 2],
    'C': [mapOper.mapAss, 3, 4, 5],
    'D': [mapOper.mapProd, [mapOper.mapSum, 1, 0], 0, 1],
    'E': 5
    }

Variables work differently in python than in C-ish languages. There are better explanations of it that I will link to later, but for now this will suffice: Variables in python are essentially C-pointers that can point to any object, which is everything in python.

demoCmd = defaultdict(list)

creates a new defaultdict(list) and assigns demoCmd to it.

demoCmd = { ... }

was intended to modify the data in the defaultdict object, demoCmd. However it reassigns the variable demoCmd to a new dict object ({} is equivalent to dict()). This wasn't a problem because demoCmd never needed to be a defaultdict object in the first place. However, if it needed to be, the data could be entered using member functions or operators like so

demoCmd = defaultdict(list)
demoCmd.add('A': [1, 2, 3])
demoCmd['B'] = [4, 5, 6]

---

Type checking with isinstance() is preferred over type() as isinstance() will match the specified class and all of it's sub classes, while type() will only match the exact class. More here.

Ergo,

def evalOpList(opList, fromSheet, row):
    if type(opList) is str:  # treat arg as a value
        return opList
    elif type(opList) is int:  # treat arg as index
        return fromSheet.cell(row, opList).value
    # ...

is more flexible as,

def evalOpList(opList, fromSheet, row):
    if isinstance(opList, str):  # treat arg as a value
        return opList
    elif isinstance(opList, int):  # treat arg as index
        return fromSheet.cell(row, opList).value
    # ...

---

The command dictionary has the structure of {key : [func , arg1, arg2, arg3, ...]}. Separating the arguments from the function as a tuple,

{key: (func, (arg1, arg2, arg3, ...))}

may seem more verbose but it will make extracting the pair simpler,

else:  # its another function
    func, arg_list = operator_tuple
    args = [evalOpList(arg, fromSheet, row) for arg in arg_list]
    return func(*args)

Allowing nesting of functions in the map commands creates an unnecessary complexity to the code. Dropping that ability allows evalOplist to be split into smaller functions.

def evaluate_command(command, from_sheet, row):
    if isinstance(command, (tuple, list)):
        return evaluate_func(func, val_list, from_sheet, row)
    else:
        evaluate_value(command, from_sheet, row)

def evaluate_value(val, from_sheet, row):
    if isinstance(val, int):
        return from_sheet.cell(row, val).value
    else:
        return command

def evaluate_func(func, val_list, from_sheet, row):
    args = [evaluate_value(val, from_sheet, row) for val in val_list]
    return func(*args)

Two problems still exist:

1. evaluate_value is not recursing into any sequences to replace indexes, which is needed for functions like sum.
2. evaluate_command is unnecessarily re-interpreting items in the mapping command for each row of the sheet. Commands could be pre-emptively interpreted into functions that take the row values and return a value.

I could walk readers through how to completely redesign the structure to pre-interpret the commands, but that seems out of codereveiw's scope. See here and here for those solutions.