python / typeshed

Collection of library stubs for Python, with static types
Other
4.32k stars 1.73k forks source link

functools.wraps does not work on class methods #10653

Open jakkdl opened 1 year ago

jakkdl commented 1 year ago

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

AlexWaygood commented 1 year ago

@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.

tamird commented 7 months ago

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

AlexWaygood commented 7 months ago

@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 commented 7 months ago

How can I help?

AlexWaygood commented 7 months ago

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

jakkdl commented 6 months ago

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 
AlexWaygood commented 6 months ago

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 commented 6 months ago

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.

srittau commented 3 months ago

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