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

Decorated bound instance method cannot be inferred correctly. #3954

Closed
huntzhan opened this issue Sep 16, 2022 · 4 comments
Closed

Decorated bound instance method cannot be inferred correctly. #3954

huntzhan opened this issue Sep 16, 2022 · 4 comments
Labels
as designed Not a bug, working as intended

Comments

@huntzhan
Copy link

Note: if you are reporting a wrong signature of a function or a class in the standard library, then the typeshed tracker is better suited for this report: https://github.com/python/typeshed/issues.

Describe the bug
A clear and concise description of what the bug is.

from typing import Callable, TypeVar
from typing_extensions import Concatenate, ParamSpec

_S = TypeVar('_S')
_P = ParamSpec('_P')
_R = TypeVar('_R')


def proxy_method(method: Callable[[_S], Callable[_P, _R]]) -> Callable[Concatenate[_S, _P], _R]:

    def new_method(_self: _S, *args: _P.args, **kwargs: _P.kwargs):
        other_method = method(_self)
        return other_method(*args, **kwargs)

    return new_method


class Foo:

    def do_something(self, x: int):
        return 42 + x


class Bar:

    def __init__(self):
        self.foo = Foo()

    @proxy_method
    def do_something(self):
        return self.foo.do_something

    def do_other(self, x: int):
        return x + 1


def debug():
    Bar.do_something
    Bar.do_other
    bar = Bar()
    print(bar.do_other(1))
    print(bar.do_something(1))

proxy_method decorates an instance method (Bar.do_something). The instance method to be decorated should return a callable (self.foo.do_something). proxy_method should return a new instance method (function) simply acting as a proxy method to the the callable. Hence, calling bar.do_something(1) is equivalent to calling bar.foo.do_something(1).

But pyright cannot infer the bound instance method correctly:

image

(bar.do_something should be inferred as bound, and should not raise a type error)

And I also notice pyright treats the "naive" instance method (function) and the decorated one differently:

image

(Bar.do_something is decorated)

image

(Bar.do_other is not decorated)

To Reproduce
Steps to reproduce the behavior.

aforementioned.

Expected behavior
A clear and concise description of what you expected to happen.

bar.do_something should be inferred as bound, and should not raise a type error, just like bar.do_other:

image

Screenshots or Code
If applicable, add screenshots or the text of the code (surrounded by triple back ticks) to help explain your problem.

def foo(self) -> str:
    return 3

inlined.

If your code relies on symbols that are imported from a third-party library, include the associated import statements and specify which versions of those libraries you have installed.

VS Code extension or command-line
Are you running pyright as a VS Code extension or a command-line tool? Which version? You can find the version of the VS Code extension by clicking on the Pyright icon in the extensions panel.

I'm running pyright v1.1.271 as a VS Code extension.

Additional context
Add any other context about the problem here.

@erictraut
Copy link
Collaborator

You're encountering an aspect of PEP 612 that is undefined. For functions, type checkers like pyright need to track whether they should or should not bind to an object when referenced through that object. Normal instance methods always bind, but static methods and functions stored to instance variables do not.

class Foo:
    def method1(s) -> None:
        ...
    
    @staticmethod
    def method2(s) -> None:
        ...

    method3 = lambda s: None

    def __init__(self):
        self.method4 = lambda s: None

f = Foo()

reveal_type(f.method1) # () -> None
reveal_type(f.method2) # (s) -> None
reveal_type(f.method3) # () -> None
reveal_type(f.method4) # (s) -> None

Pyright uses internal flags to track whether a function should be bound or not. In the example above, these flags would indicate that method1 and method3 should be bound and method2 and method4 should not.

When applying a decorator that uses a ParamSpec, it's not clear whether the intent is to preserve whether the method should be bound. It depends on the implementation of the decorator, so a type checker doesn't know. Pyright contains some heuristics to determine whether the result of applying the decorator should preserve the "needs to be bound" flag. In your code sample, it is clearing this flag when applying the @proxy_method decorator to do_something because the inferred return type of do_something is an already-bound function. Because the "needs to be bound" flag is cleared, the expression bar.do_something is assumed not to require binding. Your particular decorator implementation assumes that it will be bound, but you could have implemented it in a way where binding doesn't occur at runtime. So there's no "right answer" here for a type checker. Regardless of what behavior it assumes, it will potentially be wrong some of the time.

The way you can work around this is to provide an explicit return type annotation for do_something.

    @proxy_method
    def do_something(self) -> Callable[[int], int]:
        return self.foo.do_something

This code type checks with mypy because mypy doesn't infer return types of functions, so it treats the return type of do_something as Any.

@erictraut erictraut added the as designed Not a bug, working as intended label Sep 17, 2022
@huntzhan
Copy link
Author

@erictraut Thank you for your explanation!

Pyright contains some heuristics to determine whether the result of applying the decorator should preserve the "needs to be bound" flag.

I'm wondering if there's a work around to add the "needs to be bound" flag inside the decorator function (proxy_method), in a way pyright could know the decorated function needs to be bound? Would be great if it is possible 😊

@erictraut
Copy link
Collaborator

The Python type system currently provides no explicit way to specify whether a callable type needs to be bound to an object (as in an instance method) or a class (as in a class method). It would be possible to add some way to make that explicit, but this would require some new syntax which would need to be proposed, documented, debated, ratified, and implemented across all type checkers. I don't recall seeing this issue raised in any of the Python typing forums in the past, so I don't think it rises to the level of a problem that the broader typing community would be motivated to solve. So I think we're stuck with heuristics for now.

@huntzhan
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

2 participants