Skip to content

Pylance Typing checking error for AsyncTransaction #1015

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

Closed
eric-nguyen-cs opened this issue Jan 31, 2024 · 4 comments
Closed

Pylance Typing checking error for AsyncTransaction #1015

eric-nguyen-cs opened this issue Jan 31, 2024 · 4 comments

Comments

@eric-nguyen-cs
Copy link

Bug Report

Screenshot 2024-01-31 at 11 52 15

I am using the AsyncTransaction context manager

 async with txn_manager as _txn:
    ...

However, the Pylance VSCode extension (powered by Pyright) in Basic Type checking mode is giving a type error:
Object of type "AsyncTransaction" cannot be used with "with" because it does not implement __aenter__
Object of type "AsyncTransaction" cannot be used with "with" because it does not implement __aexit__

I believe that it is because AsyncTransaction's __aenter__ and __aexit__ are wrapped with functools.wraps(AsyncTransactionBase._enter) and functools.wraps(AsyncTransactionBase._exit) respectively

My Environment

Python Version: 3.11
Driver Version: 5.14
Operating System: macOS

@robsdedude
Copy link
Member

robsdedude commented Jan 31, 2024

Hi and thanks for reaching out.

I'm thinking this might rather be an issue with the chosen type checker (Pylance, in your case).

Reasons I'm thinking this:

  • mypy is happy with this
  • Looking at how functools.wraps is annotated in typeshed should make the type checker infer that wraps(xyz) returns the identity function (only regarding __call__). So wraps(xyz)(abc).__call__(...) should be equivalent to abc.__call__(...). In our example, the type checker should consider __aenter__ and __aexit__ to exist and be callable.

Please let me know if you disagree and why.

@eric-nguyen-cs
Copy link
Author

Thank you for your quick response

I agree what you said, the bug does seem to come from pyright and not the neo4j code: I ran the code against other type checkers (pyre-check and pytype) and it seems that only pyright raised an issue here.

However, I am wondering if the AsyncTransaction methods should be wrapped with the AsyncTransactionBase ones : the functools.wraps function's documentation seems to indicate that functools.wraps is mostly used for decorated functions. I'm not sure what the value of adding this decorator here: the docstrings could easily be moved to the AsyncTransaction methods, as the AsyncTransactionBase class is not exposed to the library consumers.

@robsdedude
Copy link
Member

You are right in that wraps does not seem to be apt here. While I could use the optional arguments to wraps to make it only copy the docs string (and maybe annotations), there's no good reason for this complication. So I cleaned up the code in the PR linked above. Thanks again for reaching out.

@eric-nguyen-cs
Copy link
Author

And thank you for quickly responding and solving the issue!

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

2 participants