Add TypedDict versions of _Details type hint

[leonhard-s]

This adds the backoff.types namespace containing the DetailsException and DetailsPredicate types which may be used to type custom handlers in user code.

Example of Pylance suggesting valid keys for this type of backoff handler

Here is a full example for illustration of what using these looks like:

import random

import backoff
from backoff.types import DetailsException


def on_backoff(details: DetailsException) -> None:
    print(f"Failed (waiting {details['wait']} s)")

def on_success(details: DetailsException) -> None:
    print(f"Succeeded after {details['tries']} attempts")

@backoff.on_exception(wait_gen=backoff.expo,
                      exception=(ValueError,),
                      on_backoff=on_backoff,
                      on_success=on_success)
def foo() -> None:
    # Cause errors ~80% of the time
    if random.random() < 0.80:
        raise ValueError()

foo()

Note: Using a plain ValueError as the exception class causes issues with Pylance. This seems isolated to this type checker and the tuple works as intended. I don't see anything wrong with the type hints used internally.

[leonhard-s]

The names DetailsException and DetailsPredicate feel very verbose but I was not able to produce better ones off the top of my head.

[bgreen-litl]

Thanks for this and especially the Pylance suggestion example.

I am persuaded by your argument that it would be better to have explicit types for the different available keys. However, I think this implementation isn't quite right because one of the "extra" keys (value) is only for the backoff.on_predicate decorator, but the other extra key (wait) is only for the on_backoff handler. In other words, both decorators share a common set of keys; the on_predicate decorator always has one extra; and if the specific handler event is on_backoff there is another extra in either the on_exception or on_predicate case. So I'm afraid there's a bit of a combinatorial explosion problem if we have to explicitly type each of these possibilities. Thoughts?

[bgreen-litl]

This seems relevant python/mypy#4617

[leonhard-s]

I guess I should have looked at the actual source code first - apologies, I'll get that fixed rightaway.

The combinatorics seem manageable for now (non-predicate, predicate, non-predicate-backoff, predicate-backoff -- if I'm not mistaking), though this will cascade if more custom fields are added to these dictionaries for other features, as you said.

I don't think the TypedDict limitation affects us since subclassing and mixins are fully supported according to Mypy, Pylance and the PEP defining them:

class Details(TypedDict):
  target: Callable[..., Any]
  args: Tuple[Any, ...]
  kwargs: Dict[str, Any]
  tries: int
  elapsed: float
  
class DetailsBackoff(Details):
  wait: float

Looking through the thread prompted me to test the current fallback types with Mypy though, which surfaced an issue: Mypy does not strictly follow conditionals, so dynamically redefining what the Details type is causes errors.

I have an idea for a workaround though, I'll look into that once the dictionary keys are fixed.

[leonhard-s]

Updated the types, they should now match what the implementation actually uses them for.

The docstrings are a little overkill since any workflow benefitting from a TypedDict hint would also have sufficient introspection to make listing the keys in the docstrings superfluous.

[leonhard-s]

A summary of the aforementioned Mypy issue:

Our TypedDict dependency workaround relies on first typing these dictionaries as Dict[str, Any] before re-casting them to proper TypedDict subclasses. If we cannot find this type in typing or typing_extensions, we leave it at that value.

Mypy does not allow this, and the --allow-redefinition flag, which would normally resolve this, currently only works with local variables, not functions or - in our case - classes/types.

The only reliable solution I was able to find was to skip the initial Dict[str, Any] type and always use the subclasses. I still want to avoid having a hard dependency for this, so I decided to set TypedDict equal to object if it cannot be found. This makes the resulting objects featureless instances, which can be used for annotations and don't hurt anyone at runtime.

I will have a look for other solutions, but this seems like it's the only way to get both Mypy and Pylance supported without requiring external dependencies.

[bgreen-litl]

Thanks for your work on this. I appreciate it especially how quick and responsive you've been. I'm trying to keep up as best I can!

In the end I think I'm not comfortable with the type "explosion" here. Like you said it's wouldn't be that unmanageable with just 4 types, but I'm afraid it's going to end up being more confusing to the user to have these types which define just one more or less key. And I think it will be hard to name them in a way that reduces this confusion. And yes, as you said, there could end up being a future feature that make these types branch again.

I'm thinking what we could do instead is something like:

class _Details(TypedDict):
  target: Callable[..., Any]
  args: Tuple[Any, ...]
  kwargs: Dict[str, Any]
  tries: int
  elapsed: float
  
class Details(_Details, total=False):
  wait: float  # this key will be present in the on_backoff handler case for either decorator
  value: Any  # this key will be present in the on_predicate decorator case

In addition to the mypy issue I linked above, I think PEP 655 is also relevant. That's in a draft state, but it identifies the above pattern as the common (if cumbersome) solution. I think it may be wise to stick to this pattern and assume there might be a better way to do it eventually.

I think we might still want to put Details in a types submodule as you suggested (even if there's only one type for now) because I like the way it separates things from the main functional API.

[leonhard-s]

I agree with your assessment; the previous solution was neither elegant nor concise enough, and there seem to be better options on the horizon.

For this version, Mypy will flag invalid keys but will silently ignore that some keys might not exist at runtime (such as "wait"). Pylance will also flag invalid keys and warns that optional keys might not be available at runtime.

[bgreen-litl]

I tested and played around with this some more, and I think I have worked out a way we can go back to falling back to Dict[str, Any] rather than object in the case that TypedDict is not available.

details_kwargs = {"total": False}

if sys.version_info >= (3, 8):
    from typing import TypedDict
else:
    # use typing_extensions if installed but don't require it
    try:
        from typing_extensions import TypedDict
    except ImportError:
        TypedDict = Dict[str, Any]
        del details_kwargs["total"]


class _Details(TypedDict):
    target: Callable[..., Any]
    args: Tuple[Any, ...]
    kwargs: Dict[str, Any]
    tries: int
    elapsed: float


class Details(_Details, **details_kwargs):
    wait: float  # this key will be present in the on_backoff handler case for either decorator
    value: Any  # this key will be present in the on_predicate decorator case

I have tested this with mypy and pylance, as well as at runtime, in each of python 3.9, python 3.6 (with type_extensions), and python 3.6 (without type extensions). Suggested completions in pylance are better with type_extensions installed (as expected) but otherwise it seems to work in all cases.

[leonhard-s]

That looks great - I overlooked that types are still valid runtime objects, this is much cleaner than via object.

[hoefling]

Although IMO it's not worth the fuzz just for supporting 3.6 and 3.7, I agree @bgreen-litl's suggestion looks very solid. @bgreen-litl once this PR lands, do you have any plans to release a 2.0a1 for the early birds out there to run type hints field tests?

[bgreen-litl]

@hoefling I just drafted a v2.0.0-alpha release with type hints and a few other things

https://github.com/litl/backoff/releases/tag/v2.0.0-alpha

[hoefling]

@bgreen-litl awesome! Do you plan to push alpha wheels to PyPI?

[DanCardin]

@bgreen-litl bump slash +1 for the idea of pushing an alpha release to pypi.