benmoran56 / esper

An ECS (Entity Component System) for Python
MIT License
537 stars 67 forks source link

`get_components` type annotation: accept and return *different* component types, not the same #60

Closed sotte closed 1 year ago

sotte commented 2 years ago

Thanks for the nice library!

As mentioned earlier, get_components has some type annotation issues.

First, the lru_cache swallows the type annotation, see https://github.com/benmoran56/esper/issues/59. Let's assume the decorator is fixed/removed.

Second, The argument *component_types: _Type[_C] says that we accept multiple component_typse, more specifically it says that they *all have the same type _Type[_C]. However, each component type is different. Same holds for the return (here without the _Type, just plain _C).

I'm not a python typing expert, but I think we would have to define multiple functions for the arity of aguments/elements to return. Something like this:

from typing import TypeVar
C1 = TypeVar("C1")
C2 = TypeVar("C2")
C3 = TypeVar("C3")

def get_components(
    self, ct1: type[C1], ct2: type[C2]
) -> list[tuple[int, tuple[C1, C2]]]:
    return []

def get_components(
    self, ct1: type[C1], ct2: type[C2], ct3: type[C3]
) -> list[tuple[int, tuple[C1, C2, C3]]]:
    return []

def get_component(
    ...

Well, of course python does not allow to overwrite functions like this :) One could hack something together or duplicate a bunch of code, but this might slow down esper too much. Maybe you or somebody has an idea how to cleanly handle this issue.

HexDecimal commented 2 years ago

You were close. Python does allow overloading function definitions using the typing.overload decorator.

from typing import TypeVar, overload
C1 = TypeVar("C1")
C2 = TypeVar("C2")
C3 = TypeVar("C3")

@overload
def get_components(
    self, ct1: type[C1], ct2: type[C2]
) -> list[tuple[int, tuple[C1, C2]]]:
    pass

@overload
def get_components(
    self, ct1: type[C1], ct2: type[C2], ct3: type[C3]
) -> list[tuple[int, tuple[C1, C2, C3]]]:
    pass

...

def get_components(...):
    ...  # Actual implementation.

Of course, this only handles typing for as many parameters as one is willing to make overloads for.

sotte commented 2 years ago

Today I learned, thanks @HexDecimal. Big fan btw!

I quickly checked. Even though I get some warnings for the various get_coponents() ("Overloaded function incompatible", "Function dedeclared type of '' must return value.") the actual type inference works as shown here: image

from typing import TypeVar, overload, List, Tuple

C1 = TypeVar("C1")
C2 = TypeVar("C2")
C3 = TypeVar("C3")

@overload
def get_components(ct1: type[C1], ct2: type[C2]) -> List[Tuple[int, Tuple[C1, C2]]]:
    pass

@overload
def get_components(
    ct1: type[C1], ct2: type[C2], ct3: type[C3]
) -> List[Tuple[int, Tuple[C1, C2, C3]]]:
    pass

def get_components(*args):
    for a in args:
        print(a)
    return [(1, [a for a in args])]

class A: pass
class B: pass

res = get_components(A, B)
id_ = res[0][0]
data = res[0][1]
HexDecimal commented 2 years ago

This would be my next iteration on the example. I've actually tested this one:

from typing import Any, List, Tuple, Type, TypeVar, overload

C1 = TypeVar("C1")
C2 = TypeVar("C2")
C3 = TypeVar("C3")

@overload
def get_components(__ct1: Type[C1], __ct2: Type[C2]) -> List[Tuple[int, Tuple[C1, C2]]]:
    pass

@overload
def get_components(
    __ct1: Type[C1], __ct2: Type[C2], __ct3: Type[C3]
) -> List[Tuple[int, Tuple[C1, C2, C3]]]:
    pass

def get_components(*args: Type[object]) -> List[Tuple[int, Any]]:
    for a in args:
        print(a)
    return [(1, tuple(a() for a in args))]

class A: pass
class B: pass

res = get_components(A, B)
id_ = res[0][0]
data = res[0][1]
assert isinstance(data, tuple)
assert isinstance(data[0], A)

The double underscore on parameters like __ct1 mark them as positional so that the main function doesn't need a **kwargs parameter. The main function should be typed even though it isn't typed in most of the linked examples.

Be sure to test the types of the return values to make sure they match the expected type. Your example was returning a list of types instead of a tuple of instances. I've added asserts to check these.

I couldn't make a return type more specific than List[Tuple[int, Any]]. The return type List[Tuple[int, Tuple[Any, ...]]] seems to cause mismatches with the overloads for some reason I can't explain.

sotte commented 2 years ago

Nice, you're getting there! :)

Same code as yours, with the following changes:

res = get_components(A, B)
id_ = res[0][0]
data = res[0][1]
reveal_type(data)
reveal_type(data[0])
reveal_type(data[1])
assert isinstance(data, tuple)
assert isinstance(data[0], A)

Different type checkers yield different results :)

❯ mypy type_overload2.py
type_overload2.py:34: note: Revealed type is "Tuple[type_overload2.A*, type_overload2.B*]"
type_overload2.py:35: note: Revealed type is "type_overload2.A*"
type_overload2.py:36: note: Revealed type is "type_overload2.B*"
❯ pyright type_overload2.py
No configuration file found.
No pyproject.toml file found.
stubPath /home/stefan/tmp/type_overload/typings is not a valid directory.
Assuming Python platform Linux
Searching for source files
Found 1 source file
/home/stefan/tmp/type_overload/type_overload2.py
  /home/stefan/tmp/type_overload/type_overload2.py:10:57 - error: Function with declared type of "List[Tuple[int, Tuple[C1@get_components, C2@get_components]]]" must return value
    Type "None" cannot be assigned to type "List[Tuple[int, Tuple[C1@get_components, C2@get_components]]]" (reportGeneralTypeIssues)
  /home/stefan/tmp/type_overload/type_overload2.py:17:6 - error: Function with declared type of "List[Tuple[int, Tuple[C1@get_components, C2@get_components, C3@get_components]]]" must return value
    Type "None" cannot be assigned to type "List[Tuple[int, Tuple[C1@get_components, C2@get_components, C3@get_components]]]" (reportGeneralTypeIssues)
  /home/stefan/tmp/type_overload/type_overload2.py:34:13 - info: Type of "data" is "Tuple[A, B]"
  /home/stefan/tmp/type_overload/type_overload2.py:35:13 - info: Type of "data[0]" is "A"
  /home/stefan/tmp/type_overload/type_overload2.py:36:13 - info: Type of "data[1]" is "B"
  /home/stefan/tmp/type_overload/type_overload2.py:21:5 - error: Overloaded function implementation is not consistent with signature of overload 1
    Type "(*args: Type[object]) -> List[Tuple[int, Any]]" cannot be assigned to type "(__ct1: Type[C1@get_components], __ct2: Type[C2@get_components]) -> List[Tuple[int, Tuple[C1@get_components, C2@get_components]]]"
      Parameter 1: type "Type[C1@get_components]" cannot be assigned to type "Type[object]"
        Type "Type[C1@get_components]" cannot be assigned to type "Type[object]"
      Parameter 2: type "Type[C2@get_components]" cannot be assigned to type "Type[object]"
        Type "Type[C2@get_components]" cannot be assigned to type "Type[object]" (reportGeneralTypeIssues)
  /home/stefan/tmp/type_overload/type_overload2.py:21:5 - error: Overloaded function implementation is not consistent with signature of overload 2
    Type "(*args: Type[object]) -> List[Tuple[int, Any]]" cannot be assigned to type "(__ct1: Type[C1@get_components], __ct2: Type[C2@get_components], __ct3: Type[C3@get_components]) -> List[Tuple[int, Tuple[C1@get_components, C2@get_components, C3@get_components]]]"
      Parameter 1: type "Type[C1@get_components]" cannot be assigned to type "Type[object]"
        Type "Type[C1@get_components]" cannot be assigned to type "Type[object]"
      Parameter 2: type "Type[C2@get_components]" cannot be assigned to type "Type[object]"
        Type "Type[C2@get_components]" cannot be assigned to type "Type[object]"
      Parameter 3: type "Type[C3@get_components]" cannot be assigned to type "Type[object]"
        Type "Type[C3@get_components]" cannot be assigned to type "Type[object]" (reportGeneralTypeIssues)
4 errors, 0 warnings, 3 infos 
Completed in 0.702sec
HexDecimal commented 2 years ago

I checked with Pyright.

Replace pass with ... in the overload functions to fix the must return value errors.

I couldn't fix the type, but I could suppress the remaining errors by replacing Type[object] with Type[Any].

The return type List[Tuple[int, Tuple[Any, ...]]] works with Pyright but not with MyPy.

Careful with reveal_type, this just tells you want the type checker thinks the type is, which may not be the names real type. Verify types with isinstance in tests.

Felecarpp commented 1 year ago

The solution above works fine. I will write a pull request.

Maybe there will be a more elegant solution in the future using TypeVarTuple but it is not yet implemented by mypy and the documentation says nothing that I read of about type[TypeVarTuple].

Components = TypeVarTuple("Components")

def get_components(
    self, *component_types: Unpack[type[Components]]
) -> Iterator[tuple[EntityID, Components]]:
    # implementation here