Refine Lambda typings #13602
Refine Lambda typings #13602
Conversation
joe4dev
added
semver: patch
joe4dev
requested review from
dfangl,
dominikschubert and
gregfurman
as code owners
| def get_invocation_lease( | ||
| self, function: Function | None, function_version: FunctionVersion | ||
| ) -> InitializationType: | ||
| ) -> Generator[InitializationType, Any, None]: |
There was a problem hiding this comment.
Think there's a bit of a conceptual mismatch here.
From the python typing.Generator docs:
If your generator will only yield values, set the
SendTypeandReturnTypetoNone:
Your current signature implies the returned Generator accepts a value via the generator.send method. This allows a caller to "send" values into generator functions, resuming execution.
However, the contextmanager protocol never sends values into generators -- instead only ever relying on next() to start/resume execution.
So the type should actually be:
| ) -> Generator[InitializationType, Any, None]: | |
| ) -> Generator[InitializationType, None, None]: |
Furthermore, we can simplify this type to an Iterator[YieldType] since the docs go on to suggest:
Alternatively, annotate your generator as having a return type of either
Iterable[YieldType]orIterator[YieldType]:
So since we only care about the YieldType (i.e InitializationType) we should simplify this typing to use the collections.abc.Iterator ABC:
| ) -> Generator[InitializationType, Any, None]: | |
| ) -> Iterator[InitializationType]: |
There was a problem hiding this comment.
Awesome 🙌 Thank you for sharing @gregfurman ✨
That's easiest to read despite PyCharm not liking it. I went ahead and replaced this and the other occurrence for get_environment
Maybe, PyCharm doesn't implement the with usage typings correctly here, yielding a warning 🤷♂️
There was a problem hiding this comment.
The IDE and AI generate unnecesarily complex or wrong stuff here:
- IDE:
Generator[Literal[InitializationType.provisioned_concurrency, InitializationType.on_demand], Any, None] - Gemini 3:
Generator[InitializationType, Any, None]- You correctly pointed out that
Anyshould beNonebecause the method does not have a return type - Question I asked it: "What are the correct typings for localstack.services.lambda_.invocation.counting_service.CountingService.get_invocation_lease?"
- You correctly pointed out that
| import contextlib | ||
| import logging | ||
| from collections import defaultdict | ||
| from collections.abc import Generator |
There was a problem hiding this comment.
note: If we decide to go with my suggestion for Iterator this should be replaced:
| from collections.abc import Generator | |
| from collections.abc import Iterator |
There was a problem hiding this comment.
Thanks for reminding to import from collections.abc and not the deprecated typings (first IDE suggestion)
Motivation
Found small typing issues.
Changes
InitializationTypeFixed Docker platform typing using StrEnumReverted due to Python 3.10 CLI incompatibility :(Generator[ExecutionEnvironment, None, None]:). PyCharm suggested this typing, but then complains upon usage. ❓ What's the right typing here?Related
Related to DRG-97