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

Annotating a decorator factory that can be applied to both classes and functions prevents pyright from correctly detecting types? #7448

Closed
ldorigo opened this issue Mar 10, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@ldorigo
Copy link
Contributor

ldorigo commented Mar 10, 2024

Note: the below is the result of a long process of debugging a library decorator that was hiding types. While I found a solution that works without changing pyright, I think the current behaviour might be incorrect and might cause errors elsewhere, so reporting it in case you want to fix.

Consider the following code:

from typing import Any, AsyncGenerator, Type, TypeVar, TypeVarTuple, cast, overload, Optional
from collections.abc import Callable


# Depending on how the generic variable is defined, I get different behaviours - see below 
C = TypeVar("C")
# C= TypeVar("C", bound=Callable[..., Any])
# C= TypeVar("C", bound=type)
# C = TypeVar("C", type, Callable[..., Any]) 


def mybeta(
    *,
    message: str = "",
) -> Callable[[C], C]:
    
    def decorator(func_or_class: C) -> C:
        if isinstance(func_or_class, type):
            print("class")

            return func_or_class
        
        def wrapper(*args: Any, **kwargs: Any) -> Any:
            print("decorated")
            return func(*args, **kwargs)
        
        return cast(C, wrapper)
    return decorator
   
@mybeta(message="test")
def testfunc2(foo: str) -> str:
    return "testfunc2"

@mybeta(message="test")
async def testfunc2async(foo: str) -> str:
    return "testfunc2async"
    

@mybeta(message="test")
async def testfunc2asyncgen(foo: str) -> AsyncGenerator[str, None]:
    yield "testfunc2asyncgen"

@mybeta(message="test")
class TestClass:
    def __init__(self, foo: str) -> None:
        self.foo = foo

func2return = testfunc2("test")

async def ba():
    asyncfunc2return = await testfunc2async("test")

    async for asyncgenfunc2return in testfunc2asyncgen("test"):
           var = asyngenfunc2return 


testclass = TestClass("test")

Now:

  • If I define C as C = TypeVar("C"): things work as expected, the variables have the expected types, except I cannot guard against decorating something that is not a class or a function.
  • C= TypeVar("C", bound=Callable[..., Any]) also works, and since classes also happen to be callables, it works for my purpose above
  • C= TypeVar("C", bound=type) also works as expected, decorating functions gives me an error in this case which is the expected behaviour
  • C = TypeVar("C", type, Callable[..., Any]) this is the offending one: for some reason, the variables (func2return, asyncfunc2return, asynggenfunc2return and testclass) are now all of type Any.

VS Code extension or command-line
Pylance, but also tested it with https://pyright-play.net/ .

@ldorigo ldorigo added the bug Something isn't working label Mar 10, 2024
@erictraut
Copy link
Collaborator

A constrained TypeVar is not the right solution for this case. A bound TypeVar is fine is the right approach. If you want to provide an upper bound that supports type or Callable, you can use a union.

C = TypeVar("C", bound=Callable[..., Any] | type)

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Mar 10, 2024
baskaryan added a commit to langchain-ai/langchain that referenced this issue Mar 28, 2024
**Description:** 

While not technically incorrect, the TypeVar used for the `@beta`
decorator prevented pyright (and thus most vscode users) from correctly
seeing the types of functions/classes decorated with `@beta`.

This is in part due to a small bug in pyright
(microsoft/pyright#7448 ) - however, the
`Type` bound in the typevar `C = TypeVar("C", Type, Callable)` is not
doing anything - classes are `Callables` by default, so by my
understanding binding to `Type` does not actually provide any more
safety - the modified annotation still works correctly for both
functions, properties, and classes.

---------

Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
Co-authored-by: Bagatur <baskaryan@gmail.com>
gkorland pushed a commit to FalkorDB/langchain that referenced this issue Mar 30, 2024
**Description:** 

While not technically incorrect, the TypeVar used for the `@beta`
decorator prevented pyright (and thus most vscode users) from correctly
seeing the types of functions/classes decorated with `@beta`.

This is in part due to a small bug in pyright
(microsoft/pyright#7448 ) - however, the
`Type` bound in the typevar `C = TypeVar("C", Type, Callable)` is not
doing anything - classes are `Callables` by default, so by my
understanding binding to `Type` does not actually provide any more
safety - the modified annotation still works correctly for both
functions, properties, and classes.

---------

Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
Co-authored-by: Bagatur <baskaryan@gmail.com>
Je-Cp pushed a commit to Je-Cp/jcp-langchain that referenced this issue Apr 2, 2024
**Description:** 

While not technically incorrect, the TypeVar used for the `@beta`
decorator prevented pyright (and thus most vscode users) from correctly
seeing the types of functions/classes decorated with `@beta`.

This is in part due to a small bug in pyright
(microsoft/pyright#7448 ) - however, the
`Type` bound in the typevar `C = TypeVar("C", Type, Callable)` is not
doing anything - classes are `Callables` by default, so by my
understanding binding to `Type` does not actually provide any more
safety - the modified annotation still works correctly for both
functions, properties, and classes.

---------

Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
Co-authored-by: Bagatur <baskaryan@gmail.com>
hinthornw pushed a commit to langchain-ai/langchain that referenced this issue Apr 26, 2024
**Description:** 

While not technically incorrect, the TypeVar used for the `@beta`
decorator prevented pyright (and thus most vscode users) from correctly
seeing the types of functions/classes decorated with `@beta`.

This is in part due to a small bug in pyright
(microsoft/pyright#7448 ) - however, the
`Type` bound in the typevar `C = TypeVar("C", Type, Callable)` is not
doing anything - classes are `Callables` by default, so by my
understanding binding to `Type` does not actually provide any more
safety - the modified annotation still works correctly for both
functions, properties, and classes.

---------

Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
Co-authored-by: Bagatur <baskaryan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants