irmen / Pyro5

Pyro 5 - Python remote objects
https://pyro5.readthedocs.io
MIT License
303 stars 36 forks source link

Use generic for expose? #63

Closed nelsyeung closed 2 years ago

nelsyeung commented 2 years ago

Is it possible to use Generic for the expose decorator? Currently, we can't get the right type hints for exposed methods.

For example:

class Warehouse:
    @expose
    def store(self, name: str, item: str) -> None:
        print("{0} stored the {1}.".format(name, item))

    ...

class Person:
    def deposit(self, warehouse: Warehouse):
        item = input("Type a thing you want to store (or empty): ").strip()
        if item:
            warehouse.store(self.name, item)

    ...

The warehouse.store(self.name, item) will give the type hint (method) store: ((...) -> Unknown) | type, which is basically useless, as I can pass in item as int and it won't complain.

irmen commented 2 years ago

What do you propose? From what I read, Generic is an abstract base class and is meant to "create generic types". I don't see how this is applicable to expose().

nelsyeung commented 2 years ago

Ah, sorry for the misunderstanding. I kind of just used the general term generics for generic types and generic functions.

What I meant was something like this:

from collections.abc import Callable
from typing import TypeVar

_T = TypeVar("_T", bound=Union[Callable, type])

def expose(method_or_class: _T) -> _T:
    ...

Here's a full minimal example, where you can see the correct type hints in action (I'm using Pyright in Vim):

from collections.abc import Callable
from typing import TypeVar, Union

_T = TypeVar("_T", bound=Union[Callable, type])

def expose(method_or_class: _T) -> _T:
    print("exposed!")
    return method_or_class

@expose
class Foo:
    def foo(self) -> None:
        print("foo")

class Bar:
    @expose
    def bar(self, bar: str) -> str:
        print(bar)
        return bar

Foo().foo()  # <- Hover your cursor over foo()
Bar().bar("a")  # <- Hover your cursor over bar("a")
irmen commented 2 years ago

I'm not familiar enough with TypeVar or the effect of _T = TypeVar("_T", bound=Union[Callable, type]) but the change seems harmless.