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

Make @cached_property generic over the value #2113

Closed
srittau opened this issue May 12, 2021 · 3 comments
Closed

Make @cached_property generic over the value #2113

srittau opened this issue May 12, 2021 · 3 comments
Labels
Milestone

Comments

@srittau
Copy link
Contributor

srittau commented May 12, 2021

Currently the following code will infer type Any for Foo().bar, although it should infer type str:

from werkzeug.utils import cached_property

class Foo:
    @cached_property
    def bar(self) -> str:
        return ""

reveal_type(Foo().bar)

Making class cached_property generic over the value solves this problem and corrects the type of a few fields like Request.base_url. I have a PR ready for this that actually removes a few instances of # type: ignore in werkzeug code (although it adds two new ones.)

@davidism
Copy link
Member

davidism commented May 12, 2021

I need to see the PR I guess, but note that we can't inherit from typing.Generic until we support only 3.9+ since there's a performance issue with it.

@srittau
Copy link
Contributor Author

srittau commented May 12, 2021

I opened #2117. It's my understanding that any performance issues with Generic should only affect the startup times, unless classes are created on the fly, since cached_property instances are only initiated on startup.

@davidism
Copy link
Member

You're right, I wasn't thinking about when cached_property was created, so it should be safe in this case.

@davidism davidism added this to the 2.0.1 milestone May 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants