17
\$\begingroup\$

I am defining a subclass of a the python dictionary object mpCmd where every item is converted to a lambda. The intended usage is that every item in the dictionary can be called with a single list argument (row) that will return a value based on row. Integers and optionally strings are considered indexes of the row. When converted to indexes, strings are converted as though they were Excel style column names.

A single integer n will become lambda row: row[n]. A tuple with a function and a sequence of indexes transforms as

(func, (n1, n2, n3)) => lambda row: func(row[n1], row[n2], row[n3])

Here are some basic examples

>>>command = mpCmd({0: 0, 'B': (sum, ([1, 2, 4],)), 'C': 'SPAM'})
>>>command[0](['Idle', 'Palin', 'Cleese', 'Chapman', 'Gilliam', 'Jones']))
'Idle'
>>>command[1]([1, 2, 3, 4, 5])
10
>>>command[2](ANYTHING)
'SPAM'

I have tested it and it works. Any advice appreciated, even if it's just confirming this is black magic.

def rmap(func, sequence):
    return [rmap(func, i) if isinstance(i, (tuple, list))
            #elif isinstance(i, dict) ???
            else func(i) 
            for i in sequence]


def name_to_index(col_name):
    """Converts Excel Style column name to zero offset index
    """
    return reduce((lambda index, char: index*26 + int(char, 36) - 9),
                  col_name, 0) - 1


class mpCmd(dict):
    """Stores user defined maps and converts them to f(vector) = scalar

    Every item stored in mpCmd will be converted to int: (lambda row: some_func)
    """

    def __init__(self, map_dict, offset=0, 
                 int_is_index=True, str_is_name=False):
        self.offset = offset
        self.int_is_index = int_is_index
        self.str_is_name = str_is_name
        super(mpCmd, self).__init__(self._convert_dict(map_dict))

    # Override setters, do no override accessors
    def __setitem__(self, key, val):
        super(mpCmd, self).__setitem__(*self._convert_item(key, val))

    def update(self, other):
        super(mpCmd, self).update(self._convert_dict(other))

    # Macro functions
    def _convert_dict(self, other):
        return {self._convert_key(key): self._convert_val(val)
                    for key, val in other.items()}    

    def _convert_item(self, key, val):
        return self._convert_key(key), self._convert_val(val)

    # actual replacement
    def _convert_val(self, val):
        if (isinstance(val, int) and self.int_is_index
                or isinstance(val, str) and self.str_is_name):
            return (lambda row, index=self._convert_key(val): row[index])

        elif isinstance(val, (tuple, list)):
            func, indexes = val
            assert callable(func)
            indexes = rmap(self._convert_key, indexes)
            return (lambda row: func(*rmap((lambda i: row[i]), indexes)))

        else:
            return (lambda *args, **kwargs: val)

    def _convert_key(self, key):
        if isinstance(key, str):
            return name_to_index(key)
        elif isinstance(key, int):
            return key - self.offset
        else:
            raise TypeError
\$\endgroup\$
3
\$\begingroup\$

I have a few points I would like to suggest.

Firstly, I would rename the name_to_index function to fit the verb-first function naming convention. Based on your other naming conventions, I would use something like convert_name_to_index.


The next thing I would do is rename your mpCmd class. The Pythonic convention is to use PascalCase instead of camelCase for class names. Also, shortening words can be handy in some places:

management --> mgt
character --> char

While, in others places it can inhibit understanding:

# Is this a comfy cat? A comfy cot? Comfy cut?
ct.description = 'Comfy'

Now, in your context it is pretty easy to know what the name is supposed to mean. However, especially in class names, its better to have too many characters than have too few. I would rename your class to something like CommandMap.


My next points touch on your convert_val function.

Python understands the conditional in your if-statement just fine (because or has a higher precidence than and). However, to the human eye, it can be ambiguous:

# Is the grouping this?
if (part1 and part2) or (part3 and part4):

# Or this?
if ((part1 and part2) or part3) and part4:

# Or this?
if (part1 and (part2 or part3)) and part4:

I would use the structure of my first example above in your code to help the statement feel less ambiguous.

The second part of convert_val function that I would change is your assert callable(func) line. The assert keyword is generally used in testing and just feels odd here. Here is how I would structure that section of code:

elif isinstance(val, (tuple, list)):
    func, indexes = val
    indexes = rmap(self._convert_key, indexes)

    if not callable(func):
        raise TypeError('{} is not callable'.format(func))

    return (lambda row: func(*rmap((lambda i: row[i]), indexes)))

My final point is in your convert_key function. If the input is not either an int or a str then you raise TypeError. As you have it now this is what prints:

>>>raise TypeError
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError

Instead of raising the class TypeError, raise an instance of type TypeError with its own message:

>>>raise TypeError('Key must be an int or a string.')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Key must be an int or a string.

Other than these suggestions, your code looks nice.

\$\endgroup\$
  • \$\begingroup\$ I don't understand why length needs to be passed to name_to_index. name_to_index('') returns -1 which would throw and error later in the code. Also your example calculates the wrong number name_to_index('BA', 2) != 52 \$\endgroup\$ – cheezsteak May 21 '14 at 14:44
  • \$\begingroup\$ I was wrong about your implementation. You have the correct implementation; I interpreted the code incorrectly. As for the using '', as long as you handle the error later on it will be fine. I just don't see the handling code in your snippet. \$\endgroup\$ – BeetDemGuise May 21 '14 at 15:14
  • \$\begingroup\$ I was expecting xlwt to handle writing to a negative index. but since that is MUCH later in the code I should handle it immediately. if not col_name raise... \$\endgroup\$ – cheezsteak May 21 '14 at 15:26
  • \$\begingroup\$ Yeah thats a good idea. Handle the error as soon as you can as it could save a lot of processing. \$\endgroup\$ – BeetDemGuise May 21 '14 at 15:28
  • \$\begingroup\$ Also your suggested try block is not effective. Notice how it errors when the lambda is called and when it is created. It's mysterious why the trace back shows an unhandled TypeError originating in the try block. But I think that is just pointing the definition of bad_lambda that block evaluated long ago without any error. \$\endgroup\$ – cheezsteak May 21 '14 at 15:30

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.