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

Pylance error when using a @retry decorator on a function #5035

Closed
hudcap opened this issue Feb 19, 2021 · 15 comments
Closed

Pylance error when using a @retry decorator on a function #5035

hudcap opened this issue Feb 19, 2021 · 15 comments

Comments

@hudcap
Copy link
Contributor

hudcap commented Feb 19, 2021

Asked on SO here with no response.
There are discussions about decorators (python/mypy#1551, python/mypy#3157) that are above my head, but it seems that my use-case was solved.

I'm not sure if I'm reporting a bug or my stupidity, but...

I have a function that runs some code based on random variables. There are some assertion checks on the random variables, so I use a @retry decorator from the retry pip package on the function. Everything works perfectly.

MWE:

from retry import retry

@retry()
def foo():
    pass

However, I started using pylance in VS Code with type checking set to "basic" and it's throwing the following error:

Argument of type "() -> None" cannot be assigned to parameter of type "_T@_Decorator"
  Type "() -> None" cannot be assigned to type "_T@_Decorator"

The stub being used is https://github.com/python/typeshed/blob/master/stubs/retry/retry/api.pyi

From my beginner's understanding, it looks right, and if I change the second line here from

_T = TypeVar("_T", bound=Callable[..., Any])
_Decorator = Callable[[_T], _T]

to

_T = TypeVar("_T", bound=Callable[..., Any])
_Decorator = Callable[[Callable[..., Any]], _T]

it's happy, which doesn't make any sense to me, since _T should not be any "stricter" than its bound.
So either there's an issue, or I'm in over my head and completely lost.

I'm new to type checking and trying to learn, so please go easy on me... :)

@srittau
Copy link
Collaborator

srittau commented Feb 19, 2021

The stubs look correct to me and mypy accepts the code above. But I may be missing something. Maybe @erictraut can help?

@erictraut
Copy link
Contributor

I'm not able to repro the error using Pylance or Pyright. Can you verify that your sample above produces an error for you?

There is one thing I see that wrong with the stub. The reply method is annotated to return type _Decorator. This is a generic type alias that accepts a single type parameter. But no type argument is provided in this case. That means it is implicitly _Decorator[Any], which is probably not what was intended. It should probably be _Decorator[_T]. If you enable the "reportMissingTypeArgument" setting in Pylance, it will flag the missing type argument.

@hudcap
Copy link
Contributor Author

hudcap commented Feb 19, 2021

My humble apologies.
I'm getting the error from my sample code, but only in my workspace.
When I open the file outside the vscode workspace there's no error (see pic below, workspace on left, no workspace on right)

image

I'll have to figure out why this is happening, but clearly it's not a typeshed issue.

Should I close this or leave it open in light of Eric's comment?

@erictraut
Copy link
Contributor

@srittau, @hauntsaninja, Out of curiosity, does mypy or stubtest have a mode where they flag missing type arguments for generic classes or type aliases? If so, would it make sense to enable that in the CI tests for typeshed? This is a common mistake that I see in typeshed stubs.

@erictraut
Copy link
Contributor

erictraut commented Feb 19, 2021

@hudcap, if you right click on retry and choose Go To Declaration, it should take you to the file where the retry symbol is declared. I'm guessing that you've configured your workspace in a way that the import resolution logic is finding some other module called retry, but you're expecting it to use the version from the bundled typeshed stubs.

@hudcap
Copy link
Contributor Author

hudcap commented Feb 19, 2021

@erictraut, it looks like my workspace is pointing to the typeshed stubs in pylance 2021.2.2, but outside the workspace it's pointing to the stubs in 2021.2.3
I restarted vscode and now the error is gone and it's pointing to 2021.2.3 🤦‍♂️
It must have partially updated in the background and something got messed up, because the stub itself doesn't seem to have changed between those versions.

Thank you everyone!
I'll leave it open for @erictraut's question

@srittau
Copy link
Collaborator

srittau commented Feb 19, 2021

I am not sure about such an option, but we should consider running the tests through pylance well, anyway.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 19, 2021

Yes, mypy has --disallow-any-generics. I'd love to turn it on, at least for stdlib (as well as the more basic --disallow-untyped-defs). The issue is probably just getting things clean on master.

@erictraut
Copy link
Contributor

I fixed all of these in stdlib stubs several months ago, but it tends to regress because it's not enforced.

@hauntsaninja
Copy link
Collaborator

It looks like @srittau added --disallow-any-generics a while back in #3288, but it also looks like that maybe isn't doing anything because https://github.com/python/mypy/blob/8780d45507ab1efba33568744967674cce7184d1/mypy/typeanal.py#L354 (which goes back to python/mypy@3549c0e, which is actually before that PR, but maybe not before whatever release?)

Note that --disallow-untyped-defs has a similar issue without --warn-incomplete-stub (see python/mypy#8917 (comment)). This looks all super messy and should be cleaned up

@erictraut
Copy link
Contributor

Perhaps this issue should be closed and several new issues should be opened:

  1. An issue in mypy related to the --disallow-any-generics and --disallow-untyped-defs checks as they apply to typeshed stubs.
  2. An issue in typeshed as a reminder to enable this check in the CI tests when it's ready.
  3. An issue in typeshed to discuss adding a CI test that uses pyright.

Does that sound right?

@JelleZijlstra
Copy link
Member

I'd love to see #3 (pyright in typeshed CI) happen; is there anything blocking that?

@srittau
Copy link
Collaborator

srittau commented Feb 22, 2021

@JelleZijlstra Probably just someone that implements this. :) But I have opened #5048 in the meantime.

@erictraut
Copy link
Contributor

I think this issue can be closed. The original issue has been addressed, and pyright is now included in the typeshed CI test suite.

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

5 participants