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

Add minimal type definitions for @retry decorator #221

Merged
merged 6 commits into from
Apr 3, 2020

Conversation

mplanchard
Copy link
Contributor

Hi!

We're considering retry libraries for some internal code on a well type annotated python3 codebase. Tenacity looks good, but an issue is the lack of type support, because wrapping a function in @retry causes its type to become degraded to Any (essentially untyped), losing the benefit of any type annotations on the decorated function.

This PR contains the minimal set of changes required to get the @retry decorator supporting typechecking properly: it defines two overloads, one for the decorator being used without arguments and another for the decorator being used as a decorator constructor. In both cases, the type signature specifies that the resulting callable has the same signature as the wrapped function.

It adds the typing backport as a requirement for python 2.7, and adds the empty py.typed file that PEP 561 specifies must be included in package data to signal to code that uses a library that the library contains type annotations.

I've done the type-annotations in code as comments, because I've found them much easier to keep up-to-date that way, but if you'd prefer, I could pull them out into a stub file instead.

There's more that could be done here with protocols to also provide hints on the various keyword arguments to retry(), but even without that, this change provides significant value for typed codebases.

I validated the signatures by doing a dev install of tenacity and running mypy on this example file:

from tenacity import retry

@retry
def foo(a: str) -> str:
    return a + "foo"

@retry()
def foob(a: str) -> str:
    return a + "foob"

a = foo  # sig: def (a: builtins.str) -> builtins.str
b = foob  # sig: def (a: builtins.str) -> builtins.str

c = foo("foo")  # builtins.str
d = foob("foo")  # builtins.str

reveal_locals()

The reveal_locals() output is shown in comments next to the variables. Without this change, the revealed type of all variables is Any.

Thanks!

@jd
Copy link
Owner

jd commented Apr 2, 2020

This looks great and pretty straight forward. Is there any validation step we can add to our CI to make sure this is not broken in the future?

@mplanchard
Copy link
Contributor Author

mplanchard commented Apr 2, 2020

It's a bit hard to use mypy itself programmatically, but there's a library called typeguard that we have used to validate types via annotations at runtime, though. It's python 3 only, so we'd have to gate the check on the tests running in python 3. I can look into figuring something out for that this evening!

@mplanchard
Copy link
Contributor Author

0e68cc9 adds some test-level validation with typeguard, run only in the python 3 test suites. I don't know of any way to check the types at runtime for python 2, but since 2 and 3 are using the same annotations, they should always be equivalent.

Copy link
Owner

@jd jd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perfect, thank you!

@jd jd changed the title Add minmal type definitions for @retry decroator Add minimal type definitions for @retry decorator Apr 3, 2020
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.

2 participants