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

Typing issues #53

Open
Dreamsorcerer opened this issue Oct 8, 2024 · 5 comments
Open

Typing issues #53

Dreamsorcerer opened this issue Oct 8, 2024 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Dreamsorcerer
Copy link
Member

The Callable should be able to be tightened up on line 43.
So, that can probably become Callable[[_TSelf[_T]], _T].

However, looking at that, it's defined the _cache as _cache: Dict[str, _T]. Wouldn't that make it impossible to use this decorator multiple times on different values? e.g. If I have a class with a URL and a port as int, then the dict type would need to be dict[str, URL | int], which would then result in both attributes returning URL | int.

So, I think the _cache annotation might need some reworking... I suspect the Generic will need to be dropped from _TSelf and the dict may need to use Any. I'm not sure there's a clear way to do it accurately and type safe.

Originally posted by @Dreamsorcerer in #50 (comment)

@Dreamsorcerer
Copy link
Member Author

Dreamsorcerer commented Oct 8, 2024

Following on from that, I tried adjusting the type of the Callable, but just seem to get errors (maybe because the class hasn't been fully defined at the time of the decorator, mypy doesn't recognise the class as matching the protocol?).

However, a quick test also revealed that the typing information is completely broken.
We need to add some assert_type()s to the tests and fix the type errors.

Adjusting the first test to:

def test_under_cached_property(propcache_module: APIProtocol) -> None:
    class A:
        def __init__(self) -> None:
            self._cache: Dict[str, int] = {}

        @propcache_module.under_cached_property
        def prop(self) -> int:
            return 1

        @propcache_module.under_cached_property
        def prop2(self) -> str:
            return "foo"

    a = A()
    assert_type(a.prop, int)
    assert a.prop == 1
    assert_type(a.prop2, str)
    assert a.prop2 == "foo"

Reveals that all properties are Any. I'd also expect to get another error about the dict type, but currently don't.

@webknjaz webknjaz added enhancement New feature or request help wanted Extra attention is needed labels Oct 8, 2024
@Dreamsorcerer
Copy link
Member Author

Hmm, the typing seems to work on aiohttp though, so I'm not sure what I'm doing differently here for it to not work...

@webknjaz
Copy link
Member

webknjaz commented Oct 8, 2024

Different MyPy version?

@Dreamsorcerer
Copy link
Member Author

I was testing locally with the same instance of mypy on each repo. assert_type() on req.rel_url in an aiohttp test works correctly, but in the above test it fails with Any.

@webknjaz
Copy link
Member

webknjaz commented Oct 8, 2024

Any future import of annotations anywhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants