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

Improve type annotation for backoff.expo #176

Merged
merged 2 commits into from
Oct 5, 2022

Conversation

hauntsaninja
Copy link
Contributor

@hauntsaninja hauntsaninja commented Oct 3, 2022

backoff.expo accepts floats. Most usage in our codebase has a floating point factor. If we want to preserve exact inference in cases where the return type is Generator[int, Any, None we could use an overload (TypeVar could work, but would be a little worse for cases involving defaults).

We also had an instance where someone used backoff.expo directly in a loop, expecting it to yield floats. This ran into issues because of the initial None yield.

`backoff.expo` accepts floats. Most usage in our codebase has a floating point `factor`. If we want to preserve exact inference in cases where the return type is `Generator[int, Any, None` we could use an overload (TypeVar could work, but would be a little worse for cases involving defaults).
backoff/_wait_gen.py Outdated Show resolved Hide resolved
@bgreen-litl bgreen-litl merged commit 52aef92 into litl:master Oct 5, 2022
@bgreen-litl
Copy link
Member

Thank you! This is merged and will be released shortly in backoff v2.2.0. I ended up changing a couple of other wait generators to hint float because they too work fine with int or float.

base: float = 2,
factor: float = 1,
max_value: Optional[float] = None
) -> Generator[Optional[float], Any, None]:
Copy link
Contributor

Choose a reason for hiding this comment

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

This unfortunately broke the typing for _WaitGenerator. I.e. you can no longer do backoff.on_exception(backoff.expo, ...) without a type error, in v2.2.0.

_WaitGenerator = Callable[..., Generator[float, None, None]]

/CC: @bgreen-litl

Copy link
Member

Choose a reason for hiding this comment

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

@cdce8p I'm not sure what you mean. Unit tests and manual tests are working for me...

Choose a reason for hiding this comment

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

see #177

Copy link
Member

Choose a reason for hiding this comment

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

@cdce8p @calpaterson Should be fixed in backoff 2.2.1. Let me know if you find otherwise

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

4 participants