benmoran56 / esper

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

`lru_cache` swallows type annotation in `get_component` #59

Closed sotte closed 2 years ago

sotte commented 2 years ago

Hey @benmoran56 , first of all thanks for this nice library. I'm really enjoying using it!

I noticed that the type annotations for get_component and get_components are not quite correct. Let's talk about get_component here, as it's easier. (get_components has the same plus another issue. I'll create a sep. issue for that one.)

get_component() is decorated with lru_cache, but the lru_cache swallows the type annotations (ref https://github.com/python/mypy/issues/5107). As a result the type of the returned component cannot be inferred. image

I type checked it with pyright, but other type checkers should yield similar results.

Potential Fix

A solution is to add type hits at type checking time (as suggested here https://github.com/python/mypy/issues/5107):

from typing import TYPE_CHECKING
from typing import TypeVar as _TypeVar
if TYPE_CHECKING:
    # workaround to prevent the lru_cache to swallow the type annotations
    F = _TypeVar("F")
    def _lru_cache(f: F) -> F:
        pass
else:
    from functools import lru_cache as _lru_cache

image

Code to reproduce

Here is the code to reproduce the issue:

class A:
    def a(self):
        print("From A.a()")

class B:
    def b(self):
        print("From B.b()")

world = World()
world.create_entity(A(), B())

# With lru_cache on get_component -> type issues
for i, a in world.get_component(A):
    print(a.a())
    # Cannot access member "a" for type "object*"
    # Member "a" is unknown (reportGeneralTypeIssues)

# Without the lru cache on get_component -> no type issues
for i, a in world.get_component(A):
    print(a.a())
benmoran56 commented 2 years ago

Sorry for leaving this hanging for such a long time. It got lost in my messages :disappointed:

This is a difficult one.. it's not really practical to remove the lru_cache without a big performance hit. I have been experimenting with a slightly different internal design that does it's own form of caching, I haven't done any testing with mypy.

I tried the potential fix, but I wasn't able to get it working. If you have any suggestions that would be great.

In lieu of a solution, and since this is still an open ticket on the mypy library, it might be easiest to just remove the typing information from the get_component/s methods for now :thinking:

sotte commented 2 years ago

I have a vendored version of esper that fixes the type annotation and uses the lru cache. At least it fixes get_component(), but get_components() can be fixed as well as outlined here.

I'll post my solution here later.