Problems with a decorator used for both methods and functions?

[pganssle]

In this PR @ncoghlan suggests that a single singledispatch decorator is not likely to work because of the way the descriptor protocol works.

Currently, what we're doing to support method variants is this, where we simply implement __get__ on VariantFunction so that it spawns a new VariantMethod if necessary. There is a problem that this implementation ends up eagerly binding many variants every time a new instance is created (see #6), but I don't think this necessitates an entirely new decorator.

I don't totally understand why a separate staticdispatchmethod is suggested rather than using the approach we've taken, so I'm opening this issue to track that to see if we've missed something.

If it does turn out that a single implementation can't work for both functions and methods, the options I see are (in order of how much I like them):

1. A method variant on variant itself, e.g. @variants.primary.method
2. A separate @variants.primarymethod decorator used only with methods.
3. A parameter passed to primary, e.g. @variants.primary(method=True)

Not sure what effect (if any) this will have on the VariantFunction.variant decorator.

[ncoghlan]

The more detailed comment is the one in python/cpython#4987 (diff)

On its own, class-definition-time wrapper generation works, but things can start to misbehave once you start composing the descriptor with other descriptors , so partialmethod and singledispatchmethod ended up as separate types that work differently as descriptors from the way partial and singledispatch do.

That said, if partial.__get__ and singledispatch.__get__ had been written this way from the beginning, I expect we wouldn't have needed dedicated *method versions.

[pganssle]

@ncoghlan Do you know why singledispatch and partial were not just rewritten to add the __get__ behavior seen in variants? It seems like it could be done without a change in user-facing behavior. I'm still having trouble assessing exactly what problems I'm buying with this approach.

Also, I don't think that what I'm doing here is class-definition-time wrapper generation, right? These wrappers are actually generated whenever the method is accessed, see:

class FuncWrapper:
    def __init__(self, func):
        self._func = func

    def __call__(self, *args, **kwargs):
        return self._func(*args, **kwargs)

    def __get__(self, obj, objtype=None):
        print('Wrapper descriptor called')
        if obj is not None:
            print('Method wrapper created')
            return  MethodWrapper(self, obj)

        return self


class MethodWrapper:
    def __init__(self, func_wrapper, parent):
        self._parent = parent
        self._func = func_wrapper
        print('Creating MethodWrapper')

    def __call__(self, *args, **kwargs):
        return self._func(self._parent, *args, **kwargs)


print('Defining func1')
@FuncWrapper
def func1():
    pass

print('Defining class1')

class class1:
    @FuncWrapper
    def func2(self):
        print(self)

print('\nCalling func1')
func1()

print('\nCreating instance1')
instance1 = class1()

print('\nAccessing func2')
instance1.func2

print('\nCalling func2')
instance1.func2()

print('\nCalling func2 again')
instance1.func2()

Running this:

$ python test_descriptor.py
Defining func1
Defining class1

Calling func1

Creating instance1

Accessing func2
Wrapper descriptor called
Method wrapper created
Creating MethodWrapper

Calling func2
Wrapper descriptor called
Method wrapper created
Creating MethodWrapper
<__main__.class1 object at 0x7ff8b67e1588>

Calling func2 again
Wrapper descriptor called
Method wrapper created
Creating MethodWrapper
<__main__.class1 object at 0x7ff8b67e1588>

Given this, the wrapper generation should be lazier, cheaper and probably cached. If I kinda squint I can imagine a scenario where something funky happens when you stack a registration decorator on the outside of one of these things, but I'm not sure I can come up with any obvious concrete examples, so it's hard to know how seriously to take it (e.g. how easy would the problems this causes be to work around?).

[ncoghlan]

Ah, yeah - what you're doing is essentially how the method variants in the standard library work.

The compatibility issue we had/have in the standard library is that partial and singledispatch historically implied staticmethod type behaviour, and there isn't any straightforward way to deprecate that and ask folks to add an explicit static method instead.

[pganssle]

Sounds good to me. Closing this issue as my concerns are alleviated.

[ncoghlan]

And clarifying for future readers: the class definition time problem I mention above is one that the closed PR against CPython suffered from - I just misinterpreted Paul's link to that as suggesting that python-variants also worked that way (it doesn't - python-variants works the same way as the accepted version of the standard library PR)