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

Mishandles self argument of wrapped method #4843

Closed
rparini opened this issue Mar 26, 2023 · 3 comments
Closed

Mishandles self argument of wrapped method #4843

rparini opened this issue Mar 26, 2023 · 3 comments
Labels
question Further information is requested

Comments

@rparini
Copy link

rparini commented Mar 26, 2023

Describe the bug
Pyright does not recognise that an instance of a class is implicitly passed to the self argument of a class method decorated with @functools.wraps.

To Reproduce
With pyright 1.1.300 and Python 3.9.13

import functools

class A:
    def meth(self, x):
        return x

class B:
    @functools.wraps(A.meth)
    def meth(self, x):
        return x + 1

b = B()
y = b.meth(x)

Results in two errors

pyright 1.1.300
/Users/rparini/Documents/examples/wraps_pyright.py
  /Users/rparini/Documents/examples/wraps_pyright.py:13:5 - error: Argument missing for parameter "x" (reportGeneralTypeIssues)
  /Users/rparini/Documents/examples/wraps_pyright.py:13:12 - error: "x" is not defined (reportUndefinedVariable)
2 errors, 0 warnings, 0 informations 

It seems to expect B.meth(b, x) rather than b.meth(x)

Expected behavior
I believe pyright should report no errors in the above. Pyright 1.1.285 does not report any errors but 1.1.298 and onwards does so I suspect this is an issue with pyright, rather than typeshed.

VS Code extension or command-line
I find an the error reported in this case with both the command line pyright 1.1.300 and Pylance

Pylance language server 2023.3.20 (pyright 93a4ef8) starting

@erictraut
Copy link
Collaborator

erictraut commented Mar 26, 2023

The change is due to a recent change in the typeshed stub file functools.pyi. Pyright doesn't have any special-case handling for functools.wraps. The change in typeshed was made to improve the handling of functools.wraps, but it does introduce regressions in some cases like the one you've pointed out here. You can see the new type definition if you right click on wraps and choose Go To Declaration. It makes use of the ParamSpec feature of the type system to capture the signature of the wrapped function or method.

I don't see an obvious way to make this case work through modification of the typeshed declaration without breaking other (more common) use cases of wraps. The only other solution I can think of is to completely ignore the typeshed definition and special case all of the functools.wraps logic within the type checker. That's something we generally try not to do because it is fragile and increases the maintenance burden of the type checker.

I'm curious what you're trying to do with this code. I don't think I've seen functools.wraps used in this manner before. Are you simply trying to copy the __doc__, __name__, 'qualname', __annotations__, etc. attributes from A.meth to B.meth but otherwise call through directly to B.meth? I'm curious why you'd want to do this. The method will no longer have the correct qualified name, for example. You're not actually creating a wrapper function here.

@erictraut erictraut added the question Further information is requested label Mar 26, 2023
@erictraut
Copy link
Collaborator

erictraut commented Mar 26, 2023

Here's a potential workaround:

class B:
    def meth(self, x: int) -> int:
        return x + 1

    # Copy docstring from A.meth
    functools.update_wrapper(meth, A.meth, assigned=["__doc__"])

@rparini
Copy link
Author

rparini commented Mar 27, 2023

Thanks very much for explaining and the suggested workaround. I had assumed the latest typeshed would be installed whenever pyright was installed so thought pyright was the cause, rather than the bundled typeshed definitions. That makes sense that you don't want to special case this in pyright.

I'm curious what you're trying to do with this code. I don't think I've seen functools.wraps used in this manner before. Are you simply trying to copy the __doc__, __name__, 'qualname', __annotations__, etc. attributes from A.meth to B.meth but otherwise call through directly to B.meth? I'm curious why you'd want to do this. The method will no longer have the correct qualified name, for example. You're not actually creating a wrapper function here.

Yes, what I'm actually trying to do is copy over doc strings, type annotations and default argument values from A.meth to B.meth.

In my actual project I have a parent Contour that consists of multiple child ComplexPaths and a Contour.integrate method that calls ComplexPath.integrate for each path in self.segments. By decorating Contour.integrate with @functools.wraps(ComplexPath.integrate) I am then able to define all my types, docstrings and defaults in ComplexPath.integrate and have them appear correctly in Sphinx for both the contour and the path

Screenshot 2023-03-27 at 19 48 07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants