Supporting functools.singledispatch with classes.
Hello all, In https://bugs.python.org/issue32380 I was hoping to add support for singledispatch with methods. Unfortunately, this cannot be achieved internally without ugly attribute or stack hacks. Therefore, I was thinking it would be nice if singledispatch supported a keyword argument of the argument index to dispatch on, thus one can say: class A: @singledispatch(arg=1) def method(self, a): return 'base' @method.register(int) def method(self, a): return 'int' The other option that could work is to define a special decorator for methods def methodsingledispatch(func): """Single-dispatch generic method decorator.""" wrapped = singledispatch(func) def wrapper(*args, **kw): return wrapped.dispatch(args[1].__class__)(*args, **kw) wrapper.register = wrapped.register update_wrapper(wrapper, func) return wrapper Since this is an API change, Ivan said I should post here to get feedback. I prefer the first design, as it is more generic and flexible. There is also the issue of classmethod and staticmethod. Since these are descriptors, I'm not sure they will work with singledispatch at all. if you do @classmethod @singledispatch def foo(cls, arg): ... You lose register on foo, breaking everything. I believe this would require changing classmethod thus is a non-starter. If you do @singledispatch @classmethod def foo(arg): ... The wrapper in singledispatch needs to be called as the __func__ in classmethod, but __func__ is readonly. So at the moment, I don't think it is possible to implement singledispatch on classmethod or staticmethod decorated functions. I look forward to people's thoughts on these issues. Cheers, Ethan Smith
On 25 December 2017 at 12:32, Ethan Smith
So at the moment, I don't think it is possible to implement singledispatch on classmethod or staticmethod decorated functions.
I've posted this to the PR, but adding it here as well: I think this is a situation very similar to the case with functools.partialmethod, where you're going to need to write a separate functools.singledispatchmethod class that's aware of the descriptor protocol, rather than trying to add the functionality directly to functools.singledispatch. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 26 December 2017 at 01:41, Nick Coghlan
On 25 December 2017 at 12:32, Ethan Smith
wrote: So at the moment, I don't think it is possible to implement singledispatch on classmethod or staticmethod decorated functions.
I've posted this to the PR, but adding it here as well: I think this is a situation very similar to the case with functools.partialmethod, where you're going to need to write a separate functools.singledispatchmethod class that's aware of the descriptor protocol, rather than trying to add the functionality directly to functools.singledispatch.
I agree with Nick here. Adding a separate decorator looks like the right approach, especially taking into account the precedent of @partialmethod. -- Ivan
Okay, if there is no further feedback, I will work on a
singledispatchmethod decorator like partialmethod.
For the future perhaps, would it not be possible to tell that the passed
argument is a descriptor/function and dispatch to the correct
implementation, thus not needing two functions for essentially the same
thing?
It seems more straightforward to make the implementation a bit more complex
to provide a single, simple API to users.
Cheers,
Ethan
On Tue, Dec 26, 2017 at 3:29 PM, Ivan Levkivskyi
On 26 December 2017 at 01:41, Nick Coghlan
wrote: On 25 December 2017 at 12:32, Ethan Smith
wrote: So at the moment, I don't think it is possible to implement singledispatch on classmethod or staticmethod decorated functions.
I've posted this to the PR, but adding it here as well: I think this is a situation very similar to the case with functools.partialmethod, where you're going to need to write a separate functools.singledispatchmethod class that's aware of the descriptor protocol, rather than trying to add the functionality directly to functools.singledispatch.
I agree with Nick here. Adding a separate decorator looks like the right approach, especially taking into account the precedent of @partialmethod.
-- Ivan
On 28 December 2017 at 04:22, Ethan Smith
Okay, if there is no further feedback, I will work on a singledispatchmethod decorator like partialmethod.
For the future perhaps, would it not be possible to tell that the passed argument is a descriptor/function and dispatch to the correct implementation, thus not needing two functions for essentially the same thing?
It seems more straightforward to make the implementation a bit more complex to provide a single, simple API to users.
"Add 'method' to the decorator name when decorating a method" is a pretty simple rule to remember - it's much easier than "Add 'arg_index=1'" (which is a comparatively arbitrary adjustment that requires a fairly in depth understanding of both the descriptor protocol and type-based function dispatch to follow). And you need the change to be explicitly opt-in *somehow*, in order to avoid breaking any existing code that relies on methods decorated with "singledispatch" dispatching on the bound class. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Fri, Dec 29, 2017 at 7:02 AM, Nick Coghlan
On 28 December 2017 at 04:22, Ethan Smith
wrote: Okay, if there is no further feedback, I will work on a singledispatchmethod decorator like partialmethod.
For the future perhaps, would it not be possible to tell that the passed argument is a descriptor/function and dispatch to the correct implementation, thus not needing two functions for essentially the same thing?
It seems more straightforward to make the implementation a bit more complex to provide a single, simple API to users.
"Add 'method' to the decorator name when decorating a method" is a pretty simple rule to remember - it's much easier than "Add 'arg_index=1'" (which is a comparatively arbitrary adjustment that requires a fairly in depth understanding of both the descriptor protocol and type-based function dispatch to follow).
And you need the change to be explicitly opt-in *somehow*, in order to avoid breaking any existing code that relies on methods decorated with "singledispatch" dispatching on the bound class.
Good points. I will start working on the singledispatchmethod implementation. ~>Ethan Smith
Cheers, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
participants (3)
-
Ethan Smith
-
Ivan Levkivskyi
-
Nick Coghlan