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

Fix a bug to make memorize no longer loss type hint #277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MacHu-GWU
Copy link

@MacHu-GWU MacHu-GWU commented Apr 7, 2023

#276

I think this is a small fix and should work from 3.5+. If user are on <3.5, the type hint won't work but the code got no impact.

Screen Shot 2023-04-07 at 2 13 43 PM

@grantjenks
Copy link
Owner

Can you move this to a type shed or type stubs file? I don’t like maintaining these in the sourcez

@MacHu-GWU
Copy link
Author

MacHu-GWU commented Apr 9, 2023

I did some research. Looks like this type of decorator + type hint issue cannot be solved by a stub file. The stub file can only define the return type of your @memoize decorator function, but not the inner wrapper of it.

Maybe consider using this decorator <https://pypi.org/project/decorator/>_?

Sorry, I am not the expert on type hint, what I proposed is something that I know would work in 3.5+.
I am happy to see that if I could do anything else?

Here's my workaround with the current version 5.4.0, hope this is helpful for other peoples:

# -*- coding: utf-8 -*-

import typing as T
from diskcache import Cache


def decohints(decorator: T.Callable) -> T.Callable:
    return decorator


class TypedCache(Cache):
    def typed_memoize(self, name=None, typed=False, expire=None, tag=None, ignore=()):
        @decohints
        def decorator(func):
            return self.memoize(name, typed, expire, tag, ignore)(func)

        return decorator


cache = TypedCache(...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants