-
Notifications
You must be signed in to change notification settings - Fork 150
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
Conversation
`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).
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]: |
There was a problem hiding this comment.
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.
Line 44 in 9a20a7f
_WaitGenerator = Callable[..., Generator[float, None, None]] |
/CC: @bgreen-litl
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #177
There was a problem hiding this comment.
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
backoff.expo
accepts floats. Most usage in our codebase has a floating pointfactor
. If we want to preserve exact inference in cases where the return type isGenerator[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.