Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making a decorator which preserves function signature #1927

Closed
sharmaeklavya2 opened this issue Jul 22, 2016 · 23 comments
Closed

Making a decorator which preserves function signature #1927

sharmaeklavya2 opened this issue Jul 22, 2016 · 23 comments
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal

Comments

@sharmaeklavya2
Copy link

sharmaeklavya2 commented Jul 22, 2016

Currently this is how I annotate decorators which don't modify the function signature:

from typing import Any, Callable, TypeVar

FuncT = TypeVar('FuncT', bound=Callable[..., Any])

def print_on_call(func: FuncT) -> FuncT:
    def wrapped(*args, **kwargs):
        print("Running", func.__name__)
        return func(*args, **kwargs)
    return wrapped # type: ignore

I have to use the # type: ignore otherwise I get this error:

decorator.py: note: In function "print_on_call":
decorator.py:9: error: Incompatible return value type (got Callable[[Any, Any], Any], expected "FuncT")

I'm wondering if there is a better way to do this which doesn't need the type: ignore. What do you all suggest?

Maybe mypy can figure out from (*args, **kwargs) that the parameter types are the same. Maybe it can also see that the return value of func is the return value of wrapped so the return type is the same, and hence the function signatures are the same.

@sharmaeklavya2
Copy link
Author

Also the type of wrapped in the error message should not be Callable[[Any, Any], Any]. It should be Callable[..., Any], isn't it?

@gvanrossum
Copy link
Member

I use a cast(FuncT, wrapper) but # type: ignore works too -- I'm not aware of a better way (though you should probably add annotations to wrapper otherwise mypy won't look inside it at all).

I do agree there are two separate bugs here:

  • def wrapper(*args, **kwds) is not considered a match for Callable[..., Any]
  • mypy seems to display the type of wrapper here incorrectly

@sixolet
Copy link
Collaborator

sixolet commented Aug 8, 2016

This looks like it's an instance of python/typing#239 (comment)

@gvanrossum
Copy link
Member

gvanrossum commented Aug 8, 2016

This looks like it's an instance of python/typing#239 (comment)
python/typing#239 (comment)

Yeah, although in this case you can do it already (since there's no change
to the signature). The problem is that the universal-varargs function isn't
seen as a match for the specific callable.

@rwbarton
Copy link
Contributor

I think that def wrapper(*args, **kwds) would be considered a match for Callable[..., Any]. But here we need it to be a match for FuncT, which is some specific Callable type chosen by the caller of print_on_call. I guess it should still be considered a match because def wrapper(*args, **kwds) purports to be able to accept any sort of arguments, but mypy doesn't use that logic I guess.

There's something a bit unsettling here because if mypy did accept wrapper as having type FuncT, there's nowhere that we check that wrapper actually calls func with the type of arguments func accepts. The call to func is accepted because func has type FuncT which is a subtype of Callable[..., Any], which can accept arbitrary arguments; but in reality func probably can't accept arbitrary arguments. This is a loophole that arises from the subtyping relationships with Any. So it's probably not really worth fixing the first bullet point of @gvanrossum's earlier comment. Instead, we should have a way of typing this more accurately, along the lines of the typing proposal mentioned above.

@gvanrossum
Copy link
Member

I follow your reasoning that we probably shouldn't fix the need for a cast or type-ignore on the return, but even if we were able to type this example using variadic type variables, the wrapper implementation would probably still use *args, **kwds so it would still require a cast or type-ignore, right?

@rwbarton
Copy link
Contributor

That's right unless you also had the ability to do something like this:

def print_on_call(func: Callable[[*Args, **Kwargs], Res]) -> Callable[[*Args, *Kwargs], Res]:
    def wrapped(*args: *Args, **kwargs: **Kwargs) -> Res:
        print("Running", func.__name__)
        return func(*args, **kwargs)
    return wrapped

Note the new syntax in the signature of wrapped.

@rwbarton
Copy link
Contributor

(To maybe clarify, Args and Kwargs are supposed to be new kinds of type variables such that [*Args, **Kwargs] as the first argument of Callable quantifies over all possible argument signatures of a function. I haven't yet looked at @sixolet's proposal to see how it compares syntactically or semantically.)

@sixolet
Copy link
Collaborator

sixolet commented Aug 10, 2016

New kinds of type variables are in fashion.

I wish the unary * and ** were valid syntax where you put them 😞 , but I don't think they are. That's a minor issue, though, syntax is fungible.

This example highlights something interesting about the whole domain. "All the same arguments", in python, is spelled with two separate special ways of argument passing, that happen to work in tandem to provide very specifically correct transferral of all positional arguments including variadic ones (*args), and all optional arguments, including both named optional arguments and implicit-shoved-into-a-dict ones (**kwargs). The transferral feels particularly happenstance, in that when you splat a list of *args into your func, they will actually quite likely correspond to specific positional or optional arguments, not just star arguments, and when you splat in the **kwargs, they will actually quite likely correspond to specific positional or optional arguments too, not just star2 arguments. While func(*args, **kwargs) does in fact correctly call func with exactly all the same arguments, and maps the arguments correctly, there's no clean typecheck-time mapping you can make about which of the arguments were part of Args and which were part of Kwargs -- they only work that way in tandem.

My unmodified proposal over there has a way of saying "all kinds of arguments, no matter what", but then the typechecking of the body of the function ends up with some erasure of types (and argument names) that I thought was unavoidable, and ends up equivalent to replacing the internal argument lists with [...] and then casting on the way out. All that would be done without the user having to say # type: ignore or anything, but it's still not ideal.

I've been trying to concoct some kind of way of resolving this problem so that I think we can typecheck the body cleanly, but I am not there yet. The closest I've come is wishing for some new even-more-magical python argument-passing syntax:

Ts = TypeVar('Ts', variadic=True)
Arg = ArgVar('Arg')

def print_on_call(func: Callable[[Expand[Arg[Ts]]], Res]) -> Callable[[Expand[Arg[Ts]]], Res]:
    def wrapped(***super_args: Ts) -> Res:
        print("Running", func.__name__)
        return func(***super_args)
    return wrapped

Or something. Wishful thinking.

I also may be misunderstanding something about @rwbarton's proposal.

@refi64
Copy link
Contributor

refi64 commented Aug 10, 2016

I wish the unary * and ** were valid syntax where you put them 😞 , but I don't think they are. That's a minor issue, though, syntax is fungible.

Does that not work with PEP 448?

@sixolet
Copy link
Collaborator

sixolet commented Aug 10, 2016

@kirbyfan64 I don't think the ones to the right of the colons in the type annotation of the function work out.

@rwbarton
Copy link
Contributor

I wish the unary * and ** were valid syntax where you put them 😞 , but I don't think they are.

Right, as far as I know they actually aren't legal.

I also may be misunderstanding something about @rwbarton's proposal.

I don't think you are and I haven't really been thinking about this very carefully. The issue you mention with *args and **kwargs not necessarily matching up with the actual keyword arguments occurred to me too after I made my comment.

@gvanrossum
Copy link
Member

@sixolet, Is this fixed by your #3113? (Not counting the better syntax you're proposing in #3157.)

@sixolet
Copy link
Collaborator

sixolet commented Apr 21, 2017 via email

@gvanrossum
Copy link
Member

Hm, the OP's example still gives

error: Incompatible return value type (got Callable[[StarArg(Any), KwArg(Any)], Any], expected "FuncT"

if you remove the # type: ignore from the last line.

@gvanrossum
Copy link
Member

Hm, I think this would be fixed by variadic type variables. Your #3113 fixes other things, like decorator factories. So I think this is not fixed and we need to keep it open until we have variadic type variables. (For which we don't seem to have a PR yet, though there's a typing issue: python/typing#193). Sorry for the confusion.

@gvanrossum
Copy link
Member

Raising priority based on internal needs.

@JukkaL
Copy link
Collaborator

JukkaL commented May 31, 2017

Lowering priority because function plugins can often be used to work around this issue.

@ilevkivskyi ilevkivskyi added bug mypy got something wrong false-positive mypy gave an error on correct code and removed feature needs discussion labels May 17, 2018
hackerkid added a commit to hackerkid/zulip that referenced this issue Nov 12, 2018
Without the cast mypy creates the following error.

Incompatible return value type (got "Callable[..., Any]",
expected "CallableT

This is a known issue.

python/mypy#1927
rishig pushed a commit to rishig/zulip that referenced this issue Nov 13, 2018
Without the cast mypy raises the following error:

Incompatible return value type (got "Callable[..., Any]",
expected "CallableT")

This is a known issue: python/mypy#1927
ruchit2801 pushed a commit to ruchit2801/zulip that referenced this issue Feb 13, 2019
Without the cast mypy raises the following error:

Incompatible return value type (got "Callable[..., Any]",
expected "CallableT")

This is a known issue: python/mypy#1927
karlicoss added a commit to karlicoss/HPI that referenced this issue May 25, 2020
ok, works with this advice python/mypy#1927 + overloads
@remort
Copy link

remort commented Apr 30, 2021

This is what we use for the sake of preserving decorated function signature:

Checking this code with MyPy:

import functools
import typing as t 

_BaseDecoratedFunc = t.Callable[..., t.Any]
DecoratedFunc = t.TypeVar('DecoratedFunc', bound=_BaseDecoratedFunc)


def stringify(i: int) -> str:
    return str(i)

def my_wrapper(func: DecoratedFunc) -> DecoratedFunc:

    @functools.wraps(func)
    def wrapper(*args: t.Any, **kwargs: t.Any) -> t.Any:
        print('in log_call()')
        result = func(*args, **kwargs)
        return result

    return t.cast(DecoratedFunc, wrapper)

wrapped_stringify = my_wrapper(stringify)

reveal_type(stringify)
reveal_type(wrapped_stringify)

...gives:

Revealed type is 'def (i: builtins.int) -> builtins.str'
Revealed type is 'def (i: builtins.int) -> builtins.str'

@JelleZijlstra
Copy link
Member

PEP 612 (ParamSpec) should fix this.

@NeilGirdhar
Copy link
Contributor

This seems to work great in the latest MyPy.

Pliner added a commit to Pliner/client_python that referenced this issue Feb 8, 2022
python/mypy#1927
Signed-off-by: Yury Pliner <[email protected]>
@rudolfbyker
Copy link

This seems to work great in the latest MyPy.

Would you please link to an example and/or documentation on this? Is it possible without cast?

@NeilGirdhar
Copy link
Contributor

NeilGirdhar commented Jun 12, 2023

Would you please link to an example and/or documentation on this? Is it possible without cast?

from functools import update_wrapper
from typing import Generic, TypeVar
from typing_extensions import ParamSpec

R_co = TypeVar('R_co', covariant=True)
P = ParamSpec('P')

class FunctionDecorator(Generic[P, R_co]):
    def __init__(self,
                 func: Callable[P, R_co]):
        super().__init__()
        update_wrapper(self, func)
        self.func = func

    def __call__(self, /, *args: P.args, **kwargs: P.kwargs) -> R_co:
        return self.func(*args, **kwargs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal
Projects
None yet
Development

No branches or pull requests