3

I'm trying to memoize using a decorator with the decorator being a class not a function, but I'm getting the error

TypeError: seqLength() takes exactly 2 arguments (1 given)

I'm guessing this has something to do with the classes, but not sure what's wrong from there.

The code:

import sys

class memoize(object):
    '''memoize decorator'''
    def __init__(self, func):
        self.func = func
        self.cache = {}
    def __call__(self, *args):
        try:
            return self.cache[args]
        except KeyError:
            value = self.func(self, *args)
            self.cache[args] = value
            return value

class collatz(object):
    def __init__(self, n):
        self.max = 1
        self.n = n
    @memoize
    def seqLength(self, n):
        if n>1:
            if n%2 == 0:
                return 1+self.seqLength(n/2)
            else:
                return 1+self.seqLength(3*n+1)
        else:
            return 1
    def maxLength(self):
        for n in xrange(1, self.n):
            l = self.seqLength(n)
            if l > self.max:
                self.max = n
        return self.max

n = int(sys.argv[1])
c = collatz(n)
print c.maxLength()
sabraham
  • 65
  • 1
  • 4

3 Answers3

2

Using a class as a decorator is tricky, because you have to implement the descriptor protocol correctly (the currently accepted answer doesn't.) A much, much easier solution is to use a wrapper function, because they automatically implement the descriptor protocol correctly. The wrapper equivalent of your class would be:

import functools

def memoize(func):
    cache = {}

    @functools.wraps(func)
    def wrapper(*args):
        try:
            return cache[args]
        except KeyError:
            value = func(*args)
            cache[args] = value
            return value
    return wrapper

When have so much state you want to encapsulate it in a class anyway, you can still use a wrapper function, for example like so:

import functools

class _Memoize(object):
    '''memoize decorator helper class'''
    def __init__(self, func):
        self.func = func
        self.cache = {}

    def __call__(self, *args):
        try:
            return self.cache[args]
        except KeyError:
            value = self.func(*args)
            self.cache[args] = value
            return value

def memoize(func):
    o = _Memoize(func)
    @functools.wraps(func)
    def wrapper(*args):
        return o(*args)
    return wrapper
Thomas Wouters
  • 130,178
  • 23
  • 148
  • 122
2

This is confusing, syntactically. It's not clear if self.func is part of your memoize or a separate function that's part of some other object of some other class. (You mean the latter, BTW)

        value = self.func(self, *args)

Do this to make it clear that the_func is just a function, not a member of the memoize class.

        the_func= self.func
        value= the_func( *args )

That kind of thing prevents confusion over the class to which self. is bound.

Also, please spell it Memoize. With a leading capital letter. It is a class definition, after all.

S.Lott
  • 384,516
  • 81
  • 508
  • 779
-1

A decorator is just syntactic sugar for foo = decorator(foo), so in this case you're ending up making the self of seqLength be memoize instead of collatz. You need to use descriptors. This code works for me:

class memoize(object):
    '''memoize descriptor'''
    def __init__(self, func):
        self.func = func

    def __get__(self, obj, type=None):
        return self.memoize_inst(obj, self.func)

    class memoize_inst(object):
        def __init__(self, inst, fget):
            self.inst = inst
            self.fget = fget

            self.cache = {}

        def __call__(self, *args):
            # if cache hit, done
            if args in self.cache:
                return self.cache[args]
            # otherwise populate cache and return
            self.cache[args] = self.fget(self.inst, *args)
            return self.cache[args]

More on descriptors:

http://docs.python.org/howto/descriptor.html#descriptor-example

AdamKG
  • 13,678
  • 3
  • 38
  • 46
  • 2
    This... doesn't actually work correctly. It stores the *last* instance the memoize instance was retrieved from, so if you do, for example, 'm1 = o1.attr; m2 = o2.attr', calling 'm1' will actually call the method with 'o2' as "self". For this to work correctly, the `__get__` method has to return a *new* object. But better yet is just making the decorator a function. It's much easier to get right. – Thomas Wouters Jun 04 '13 at 11:35
  • Thanks! Fixed the example in the answer, demonstration of the bug & fixed impl here: https://gist.github.com/AdamG/6049318 – AdamKG Jul 21 '13 at 18:00