Return original function from `@primary_func.variant("name")` decorator
moshez opened this issue · 8 comments
In
variants/src/variants/_variants.py
Line 53 in 1c2bf41
self
rather than returning vfunc
. I'm not sure this has utility: we don't expect to register variants on variants, and this means that there are subtle things that might break if you add @...variant()
decorator to an existing function.Sorry, forgot about this. I think the main reason that the .variant
decorator "consumes"' the function it decorates is to avoid the need to actively delete the variant functions from the top level namespace.
The documentation tends to encourage the style like this:
@variants.primary
def func():
...
@func.variant("some_variant")
def func():
...
That style would be broken if the variant decorator were converted to a pure "registration" decorator, and I think it would possibly be the wrong design decision anyway.
Fair enough, my use case is not that common, and would also be satisfied if variant
accepted an optional second parameter so that I could do
@variants.primary
def func():
pass
def func_I_need_later():
pass
@func.variant("some_variant", func_I_need_later)
Because
@func.variant("some_variant")(func_I_need_later)
feels a bit cumbersome.
Ping?
@pganssle let me know if having a second optional parameter for the function (like functools.singledispatch
) makes sense to you. Happy to put together a PR if this looks like a good idea.
@moshez Sorry for the delay, I'm super swamped. I don't really understand what you are asking for here.
@func.variant("some_variant", func_I_need_later)
Because
@func.variant("some_variant")(func_I_need_later)
Are the @
symbols here accidentally included? Did you mean func.variant("some_variant", func_I_need_later)
?
If so, I am sort of on the fence about it, leaning against. On the one hand, it's not much to support an alternate version of this thing that allows you to attach already extant functions, but on the other hand, I'm not sure I see the benefit of it?
func.variant("some_variant", func_I_need_later)
func.variant("some_variant")(func_I_need_later)
The two are the exact same number of characters (though ()
both require the shift key on US keyboards and ,
don't, admittedly).
I would also expect you to just sort of be able to do func.some_variant = func_I_need_later
in the simplest case, no? (I don't think this would work with methods, and it probably wouldn't work properly with the documentation plugin, but you'd get many of the other benefits without a dependency).
In any case, the cost is not that high (adds a bit of complexity to the interface, prevents us from using the "second positional argument" for other more interesting stuff), but I don't think the benefit is either. If you have a very compelling reason we can do it, but otherwise I'd rather leave things as is.
You said you didn't understand, but it seems you understand perfectly. I think this makes sense -- it's a niche-case, and the extra set of parens is not the end of the world.
You said you didn't understand, but it seems you understand perfectly.
Ah, yeah, I was just confused by the extraneous @
s, and thought you might have been asking for something else.