typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

functools.wraps does not work on class methods

Open jakkdl opened this issue 2 years ago • 9 comments

We recently hit an error using @wraps on a class method in Trio: https://github.com/python-trio/trio/issues/2775#issuecomment-1702892474 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. https://github.com/python/typeshed/issues/6347

jakkdl avatar Sep 02 '23 10:09 jakkdl

@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 https://github.com/python/mypy/pull/16009, as an example.

AlexWaygood avatar Sep 02 '23 10:09 AlexWaygood

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

tamird avatar Feb 23 '24 09:02 tamird

@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

AlexWaygood avatar Feb 23 '24 09:02 AlexWaygood

How can I help?

tamird avatar Feb 23 '24 10:02 tamird

How can I help?

You could open an experimental draft PR against mypy reverting https://github.com/python/mypy/commit/0dd4b6f7576be3d3857fecefb298decdf0711ac7 and see what mypy_primer says

AlexWaygood avatar Feb 23 '24 10:02 AlexWaygood

With https://github.com/python/mypy/pull/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 

jakkdl avatar Mar 18 '24 14:03 jakkdl

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?

AlexWaygood avatar Mar 18 '24 14:03 AlexWaygood

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.

jakkdl avatar Mar 19 '24 08:03 jakkdl

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

srittau avatar Jun 24 '24 09:06 srittau