Use func.__qualname__ instead of func.__name__

[unho]

By using func.__qualname__ instead of func.__name__ we can apply the decorator to methods and get unique keys including the class' name.

For example if we have two classes with a method with the same name, and we apply @cache_memoize to both of them the same cache key will be used, unless we explicitly use prefix. But IMHO doing as suggested is a safe default.

Just noticed that for Python 2 this would require adding some dependency like https://pypi.org/project/qualname/

[peterbe]

Awesome! Go for it. It's a common pattern to have extra requirements in the setup.py for Py 2.

[peterbe]

@Geekfish is this something that interests you?

[Geekfish]

Yeah, sounds interesting, would be happy to pick it up during a local hacknight in the coming weeks if you're looking for help. I didn't know about func.__qualname__, I should start using it in other places as well!

[Geekfish]

I looked into this today and very accidentally, I ran into an interesting issue when running the tests.

So one of the tests is defined like this:

def test_cache_memoize_different_functions_same_arguments():

    calls_made_1 = []
    calls_made_2 = []

    @cache_memoize(10)
    def function_1(a):
        calls_made_1.append(a)
        return a * 2

    @cache_memoize(10)
    def function_2(a):
        calls_made_2.append(a)
        return a * 3
    # ...
    assert function_2(100) == 300
    # ...
    # If you set the prefix, you can cross wire functions.
    # Note sure why you'd ever want to do this though

    @cache_memoize(10, prefix=function_2.__name__)
    def function_3(a):
        raise Exception

    assert function_3(100) == 300

The last bit is perhaps not the most useful test, but I tried to keep it by changing it to what I assumed would work:

    @cache_memoize(10, prefix=qualname(function_2))
    def function_3(a):
        raise Exception

The expected pre-hashed key for function_2 and function_3 called with param 100 would be:

cache_memoizetest_cache_memoize_different_functions_same_arguments.<locals>.function_2100

However, the actual pre-hashed key for function_3 is:

cache_memoizecache_memoize.<locals>.decorator.<locals>.inner100

So it looks like the reference to function_3 is not the one of the original, but rather that of the function that warps it (the inner function in the memoize decorator)

This all links back to the issue described here:
wbolster/qualname#4

This is not really an issue with qualname, but it is an issue with how wraps works in python2.

The suggestion in the issue above is to use the 3rd party wrapt library when defining decorators. However, while this will fix it for our tests, it will not protect the end users, the majority of who will most likely be using wraps in their decorators.

So for example, if we were to use a random django decorator underneath memoize:

@cache_memoize(10)
@require_http_methods(["GET"])
def some_view(request):
    return "potato"


@cache_memoize(10)
@require_http_methods(["GET"])
def some_other_view(request):
    return "tomato"


class FakeGetRequest:
    method = "GET"


def test_views():
    request = FakeGetRequest()
    assert some_view(request) == "potato"
    assert some_other_view(request) == "tomato"

... the test above would fail because both functions would evaluate to

cache_memoizerequire_http_methods.<locals>.decorator.<locals>.inner<tests.test_cache_memoize.FakeGetRequest instance at ...>

and both calls would return "potato".

So, TL;DR:
After looking into it (and learning a few things about qualname), I think that swapping to qualname and having support for < 3.3 might not be a good idea :)

[peterbe]

With the "potato" and "tomato" example above, if you keep good old __name__, does it work? I.e. do you NOT get "potato" both times?

Perhaps an outcome of all of this is to put in a blurb in the README. Something like, ...

### Word of warning!!
If you use this decorator on two methods in two different classes that both have the same name, you can run into problems. If you use Python3 you can solve it by managing the prefix manually:

    class SomeClass:

        @cache_memoize(10, prefix='SomeClass.get')
        def get(self, a, b):
            ...

or something like that. What do you think?

Also, perhaps if you're going to be using cache_memoize on class methods, perhaps we should define the whole things differently. E.g.

from cache_memoize import cache_memoize, cache_memoize_method

@cache_memoize(60)
def foo(a, b):
    return a + b

class Klass:

    @cache_memoize_method(60)
    def foo(self, a, b):
        return a * b

It's not entirely clear how differently a decorator should work for functions vs. methods to work the best possible way.

[Geekfish]

Yeap, current __name__ passes for the example test, you get "potato", then "tomato"!

I think given this:
https://pythonclock.org/
and this:
https://python3statement.org/

it may be a good idea to answer whether you would:

• care about having the improvement backported to support python2.7 enough to justify having an entire new interface for methods (which may need some serious inspection kung-fu paired with special cases),
• want to only improve the python3 version and still maintain python2.7 support on newer versions, but with potentially reduced features (a bit ugly, but some libraries do it so to not completely kill off support),
• be happy to drop support for 2.7 and ship further improvements for python3 only (a significant decision also)

I'm obviously biased in this, since I currently mostly work with python3 😄

[peterbe]

I'm obviously biased in this, since I currently mostly work with python3

Who doesn't?!

Since I don't use Python 2 for anything (neither side-projects or work projects) I've kinda spaced out at this discussion and didn't really appreciate how soon Python 2 (support) goes away.

Right now we're not suffering. The topic at hand isn't a desperate bug that needs to be fixed. So perhaps the best thing to do is to just park this and park it in the back of our minds. I don't think our code would greatly improve or benefit from dropping Python 2.

Or, we can just go ahead and just do it. Change the version number to 0.2.0, remove anything Python 2ish (e.g. classifiers in setup.py, .travis.yml, tox.ini) and put a mention of it in the README.
If some poor sucker is stuck on Python 2 and already depend on this project, the files on PyPI won't go away and it's unlikely that those people would get too sour if they can't have the new latest and greatest features.

[Geekfish]

Right now we're not suffering.

Agreed. I think it would be nice to upgrade, as you said, regardless. Since __qualname__ is a "breaking change" it could easily fit in this, and it's a nice improvement as users will need have one less concern in their heads.

I'll open a separate issue for the upgrade, and we can come back to this as well.

[akx]

Now that #26 was merged, this seems relevant again.

[peterbe]

Yes, @Geekfish would it be a lot easier this time around?

[Geekfish]

Yeap, I think it should now be just a drop in replacement :)