Repository navigation
Mark the package as PEP 561 compatible so that type info is available to mypy #221
Description
Is your feature request related to a problem? Please describe.
Type checking can be useful both for productivity (autocomplete is better with it) and for correctness (validating things are being used correctly). The aws_lambda_powertools.utilities.typing submodule is neat, but it seems it's currently focused for use with IDEs, and seemingly doesn't work with mypy for type checking:
# test.py
from aws_lambda_powertools.utilities.typing import LambdaContext$ mypy test.py
test.py:2: error: Skipping analyzing 'aws_lambda_powertools.utilities.typing': found module but no type hints or library stubs
from aws_lambda_powertools.utilities.typing import LambdaContext
^
test.py:2: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)
Describe the solution you'd like
The link in the message links to a discussion about making PEP 561 compatible packages. It seems the most relevant one to this library is to add a py.typed file to the package (that is, an empty aws_lambda_powertools/py.typed file), which should allow mypy to automatically take advantage of existing type hints for LambdaContext and elsewhere.
Describe alternatives you've considered
An alternative discussed above is stub files next to the existing ones, but this seems like more work than necessary given there's already type annotations inline in the code.
Additional context
N/A
Activity
heitorlessa commentedon Dec 7, 2020
Hey @huonw - Thanks for flagging this, I'm mostly new to mypy and assumed type hints were enough.
Is adding
py.typedinto the root of the project all we need to do? If so, it's a no brainier indeed.I've just run
mypyagainst the code base and seem to have picked up quite a lot of issues - If we need to fix them first before addingpy.typed, it might take a week or two.gmcrocetti commentedon Dec 7, 2020
Hi @heitorlessa. There's no need to fix all errors, although reasonable. Some errors are false-positives due to mypy bugs.
IMO the following steps (in this order) are necessary to completely solve this issue:
py.typedfile;mypyinto CI pipeline.huonw commentedon Dec 7, 2020
Yeah, I've been tripped up by that too!
As @gmcrocetti hints, it also needs to be marked for inclusion so that it appears in the package directory once installed. The mypy docs discuss how to do this with
setuptools, and python-poetry/poetry#1338 (comment) looks like it has an example of doing so for poetry.I'm personally unclear how well MyPy will work for users if the library itself doesn't pass type checking, but maybe it'll be okay.
gmcrocetti commentedon Dec 8, 2020
Yeap, I'm also in doubt about the behavior of mypy but at least for your example it worked very well (although it's very simple).
My recommendation of running mypy in CI is due to that. Maybe we can skip for now ?
heitorlessa commentedon Dec 11, 2020
Understood! Thanks for the quick PR @gmcrocetti - Perhaps it's possible to do a pip install from
developbranch using (git+https) then runningmypyto see if that has any effect.I'll tackle #238 first as it seems to be a bug, then I'll get back to this next to go through all mypy errors + add mypy in the CI
huonw commentedon Dec 14, 2020
Awesome! It seems to mostly work 🎉 I can remove the
type: ignoreannotations associated with our use of this library, which is great.$ pip install git+https://github.com/awslabs/aws-lambda-powertools-python.git $ pip freeze | grep powertools aws-lambda-powertools @ git+https://github.com/awslabs/aws-lambda-powertools-python.git@3ac1ae3d28f66095c07c43c6912a4a8cee09a021It looks like there's still a bit of room to improve, potentially due to the errors flagged above in https://github.com/awslabs/aws-lambda-powertools-python#issuecomment-739943216. For instance:
some imports don't work perfectly:
with
@tracer.capture_lambda_handlerand@logger.inject_lambda_context(and others, potentially), the final decorated function has the wrong type, rather thanCallable[[Dict[str, Any], LambdaContext], Any]orCallable[[Dict[str, Any], LambdaContext], Dict[str, Any]]as it should be (I think?), which we notice because we've got our own decorator we use on our handlers:heitorlessa commentedon Mar 8, 2021
Hey - Sharing an update here that we haven't been able to work on this yet but we will after idempotency utility is GA.
huonw commentedon Mar 8, 2021
Great, thanks for the update!
Coincidentally, I came across this today with
@tracer.capture_method. If you're looking for priorities, the decorators might be the highest value, because if they're not typed (correctly) then every call of the decorated function will also be untyped.Unfortunately, typing decorators seems to be quite awkward, and we've introduced a stub to our project along the lines of:
heitorlessa commentedon Mar 8, 2021
hmmm it sounds like there's a lot I have to learn from MyPy to get this right -- Would you be up to contribute the types you need the most?
It'd be a great help to us both in terms of learning and for everyone who depends on it -- That way we can include in the next release instead of waiting until we free out time to do the whole thing properly.
Happy to help with any contribution questions you might have too