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

functools.wraps does not work on class methods #10653

Open
jakkdl opened this issue Sep 2, 2023 · 9 comments
Open

functools.wraps does not work on class methods #10653

jakkdl opened this issue Sep 2, 2023 · 9 comments

Comments

@jakkdl
Copy link

jakkdl commented Sep 2, 2023

We recently hit an error using @wraps on a class method in Trio: python-trio/trio#2775 (comment) when checked with pyright. It works fine on mypy, but I'm guessing they might be special-casing wraps usage while pyright doesn't.

Minimal repro (python 3.11):

from functools import wraps
from typing import assert_type


def foo(param: int) -> str:
    """docstring"""
    return ""

@wraps(foo)
def my_function(blah: int) -> str:
    return 'foo'

# works fine
assert_type(my_function(5), str)

class MyOtherClass:
    def bar(self, param: int) -> str:
        """docstring"""
        return ""

class MyClass:
    @wraps(MyOtherClass.bar)
    def my_method(self, blah: int) -> str:
        return 'foo'

instance = MyClass()
# works in mypy, but not pyright
assert_type(instance.my_method(5), str)

mypy 1.5.1 works without any issue, pyright 1.1.325 gives

./test.py
  ./test.py:26:13 - error: Argument missing for parameter "blah" (reportGeneralTypeIssues)
  ./test.py:26:13 - error: "assert_type" mismatch: expected "str" but received "Unknown" (reportGeneralTypeIssues)
2 errors, 0 warnings, 0 informations 

This smells a lot like the problem with getting functools.cache to work both with methods and functions, see e.g. #6347

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 2, 2023

@erictraut's PR #6670 changed functools.wraps to use ParamSpec. However, we found that this change only caused regressions for mypy users (likely at least in part because mypy at the time had much less complete support for ParamSpec than pyright did). As a result, mypy currently reverts that change to functools.wraps as part of every typeshed sync that it does. See the fifth commit in python/mypy#16009, as an example.

@tamird
Copy link
Contributor

tamird commented Feb 23, 2024

@AlexWaygood do you think it's appropriate to stop reverting that change on an ongoing basis now?

@AlexWaygood
Copy link
Member

@AlexWaygood do you think it's appropriate to stop reverting that change on an ongoing basis now?

Last I checked it still caused a lot of false positives for mypy users, but it might be worth checking again

@tamird
Copy link
Contributor

tamird commented Feb 23, 2024

How can I help?

@AlexWaygood
Copy link
Member

How can I help?

You could open an experimental draft PR against mypy reverting python/mypy@0dd4b6f and see what mypy_primer says

@jakkdl
Copy link
Author

jakkdl commented Mar 18, 2024

With python/mypy#16942 being merged the behavior is now similar between mypy (if installing directly from master/) and pyright:

$ mypy test.py                                       
foo.py:28: error: Missing positional argument "blah" in call to "__call__" of "_Wrapped"  [call-arg]
foo.py:28: error: Argument 1 to "__call__" of "_Wrapped" has incompatible type "int"; expected "MyClass"  [arg-type]
Found 2 errors in 1 file (checked 1 source file)

$ pyright test.py                                    
/tmp/mypy_wraps/foo.py
  /tmp/mypy_wraps/foo.py:28:13 - error: Argument missing for parameter "blah" (reportCallIssue)
  /tmp/mypy_wraps/foo.py:28:13 - error: "assert_type" mismatch: expected "str" but received "Unknown" (reportAssertTypeFailure)
2 errors, 0 warnings, 0 informations 

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 18, 2024

Oh wait, does that mean that it's fixed for both type checkers, @jakkdl? Or that both mypy and pyright now have the bug, whereas previously it was only pyright?

@jakkdl
Copy link
Author

jakkdl commented Mar 19, 2024

Both have the bug now, in that mypy no longer deviates from typeshed - which doesn't handle class methods. I think I confused myself at some point too in the mypy issue.

I don't remember enough details from #6347 to figure out if this has the same issue and can't be resolved without Intersection, or if it's a simpler case.

@AlexWaygood AlexWaygood reopened this Mar 19, 2024
srittau added a commit to srittau/typeshed that referenced this issue Jun 20, 2024
These tests demonstrate the issue described in python#10653.
JelleZijlstra pushed a commit that referenced this issue Jun 22, 2024
These tests demonstrate the issue described in #10653.
@srittau
Copy link
Collaborator

srittau commented Jun 24, 2024

Also discussed on discuss.python.org: https://discuss.python.org/t/making-functions-subscriptable-at-runtime/26463/31

max-muoto pushed a commit to max-muoto/typeshed that referenced this issue Sep 8, 2024
These tests demonstrate the issue described in python#10653.
Qalthos added a commit to ansible/molecule that referenced this issue Oct 23, 2024
The decorators are a bit more complex here than in state.py, so we need
a slightly more complicated hint.

_Ideally_, we would be able to use `Callable[Concatenate[HasConfig, P],
R]` and not have to fish the `HasConfig` out of `args`, but there are
some complicated issues with wrapped functions (see
python/typeshed#10653 for more context):

```
error: Incompatible return value type (got "_Wrapped[[HasConfig, **P], R, [HasConfig, **P], R]", expected "Callable[[HasConfig, **P], R]")  [return-value]
```

---------

Co-authored-by: Sorin Sbarnea <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants