Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Mark the package as PEP 561 compatible so that type info is available to mypy #221

Closed
huonw opened this issue Nov 26, 2020 · 9 comments
Closed
Assignees
Labels
feature-request feature request help wanted Could use a second pair of eyes/hands

Comments

@huonw
Copy link
Contributor

huonw commented Nov 26, 2020

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

@huonw huonw added feature-request feature request triage Pending triage from maintainers labels Nov 26, 2020
@heitorlessa heitorlessa added help wanted Could use a second pair of eyes/hands and removed triage Pending triage from maintainers labels Dec 7, 2020
@heitorlessa
Copy link
Contributor

Hey @huonw - Thanks for flagging this, I'm mostly new to mypy and assumed type hints were enough.

Is adding py.typed into the root of the project all we need to do? If so, it's a no brainier indeed.

I've just run mypy against the code base and seem to have picked up quite a lot of issues - If we need to fix them first before adding py.typed, it might take a week or two.


mypy aws_lambda_powertools
aws_lambda_powertools/utilities/data_classes/common.py:34: error: Incompatible return value type (got "Optional[Any]", expected "bool")
aws_lambda_powertools/tracing/extensions.py:11: error: Skipping analyzing 'aws_xray_sdk.ext.aiohttp.client': found module but no type hints or library stubs
aws_lambda_powertools/logging/lambda_context.py:49: error: "object" has no attribute "function_name"
aws_lambda_powertools/logging/lambda_context.py:50: error: "object" has no attribute "memory_limit_in_mb"
aws_lambda_powertools/logging/lambda_context.py:51: error: "object" has no attribute "invoked_function_arn"
aws_lambda_powertools/logging/lambda_context.py:52: error: "object" has no attribute "aws_request_id"
aws_lambda_powertools/utilities/data_classes/sqs_event.py:78: error: Return type "Optional[SQSMessageAttribute]" of "__getitem__" incompatible with return type "SQSMessageAttribute" in supertype "dict"
aws_lambda_powertools/utilities/data_classes/sqs_event.py:78: error: Return type "Optional[SQSMessageAttribute]" of "__getitem__" incompatible with return type "SQSMessageAttribute" in supertype "Mapping"
aws_lambda_powertools/utilities/data_classes/sqs_event.py:80: error: Argument 1 to "SQSMessageAttribute" has incompatible type "SQSMessageAttribute"; expected "Dict[str, Any]"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:303: error: Name 'sms_message' already defined on line 291
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:303: error: "Callable[[CustomMessageTriggerEventResponse], str]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:309: error: Name 'email_message' already defined on line 295
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:309: error: "Callable[[CustomMessageTriggerEventResponse], str]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:315: error: Name 'email_subject' already defined on line 299
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:315: error: "Callable[[CustomMessageTriggerEventResponse], str]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:483: error: Name 'claims_to_add_or_override' already defined on line 470
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:483: error: "Callable[[ClaimsOverrideDetails], Optional[Dict[str, str]]]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:489: error: Name 'claims_to_suppress' already defined on line 474
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:489: error: "Callable[[ClaimsOverrideDetails], Optional[List[str]]]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:494: error: Name 'group_configuration' already defined on line 478
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:494: error: "Callable[[ClaimsOverrideDetails], Optional[GroupOverrideDetails]]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:620: error: Name 'challenge_name' already defined on line 608
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:620: error: "Callable[[DefineAuthChallengeTriggerEventResponse], str]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:626: error: Name 'fail_authentication' already defined on line 612
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:626: error: "Callable[[DefineAuthChallengeTriggerEventResponse], bool]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:631: error: Name 'issue_tokens' already defined on line 616
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:631: error: "Callable[[DefineAuthChallengeTriggerEventResponse], bool]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:706: error: Name 'public_challenge_parameters' already defined on line 694
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:706: error: "Callable[[CreateAuthChallengeTriggerEventResponse], Dict[str, str]]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:713: error: Name 'private_challenge_parameters' already defined on line 698
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:713: error: "Callable[[CreateAuthChallengeTriggerEventResponse], Dict[str, str]]" has no attribute "setter"
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:722: error: Name 'challenge_metadata' already defined on line 702
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py:722: error: "Callable[[CreateAuthChallengeTriggerEventResponse], str]" has no attribute "setter"
aws_lambda_powertools/utilities/validation/jmespath_functions.py:5: error: Skipping analyzing 'jmespath': found module but no type hints or library stubs
aws_lambda_powertools/utilities/parameters/base.py:17: error: Need type annotation for 'DEFAULT_PROVIDERS' (hint: "DEFAULT_PROVIDERS: Dict[<type>, <type>] = ...")
aws_lambda_powertools/utilities/parameters/base.py:38: error: Unsupported right operand type for in ("None")
aws_lambda_powertools/utilities/parameters/base.py:38: error: Value of type "None" is not indexable
aws_lambda_powertools/utilities/parameters/base.py:80: error: Value of type "None" is not indexable
aws_lambda_powertools/utilities/parameters/base.py:89: error: Incompatible types in assignment (expression has type "Union[Dict[Any, Any], bytes, None]", variable has type "str")
aws_lambda_powertools/utilities/parameters/base.py:91: error: Unsupported target for indexed assignment ("None")
aws_lambda_powertools/utilities/parameters/base.py:141: error: Value of type "None" is not indexable
aws_lambda_powertools/utilities/parameters/base.py:144: error: Incompatible types in assignment (expression has type "Dict[str, str]", variable has type "Dict[str, Union[str, bytes, Dict[Any, Any], None]]")
aws_lambda_powertools/utilities/parameters/base.py:144: note: "Dict" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
aws_lambda_powertools/utilities/parameters/base.py:144: note: Consider using "Mapping" instead, which is covariant in the value type
aws_lambda_powertools/utilities/parameters/base.py:150: error: Incompatible types in assignment (expression has type "str", variable has type "Tuple[str, Optional[str]]")
aws_lambda_powertools/utilities/parameters/base.py:151: error: Argument 1 to "get_transform_method" has incompatible type "Tuple[str, Optional[str]]"; expected "str"
aws_lambda_powertools/utilities/parameters/base.py:155: error: Invalid index type "Tuple[str, Optional[str]]" for "Dict[str, Union[str, bytes, Dict[Any, Any], None]]"; expected type "str"
aws_lambda_powertools/utilities/parameters/base.py:155: error: Argument 1 to "transform_value" has incompatible type "Union[str, bytes, Dict[Any, Any], None]"; expected "str"
aws_lambda_powertools/utilities/parameters/base.py:157: error: Unsupported target for indexed assignment ("None")
aws_lambda_powertools/utilities/parameters/base.py:159: error: Incompatible return value type (got "Dict[str, Union[str, bytes, Dict[Any, Any], None]]", expected "Union[Dict[str, str], Dict[str, Dict[Any, Any]], Dict[str, bytes]]")
aws_lambda_powertools/tracing/tracer.py:10: error: Skipping analyzing 'aws_xray_sdk': found module but no type hints or library stubs
aws_lambda_powertools/tracing/tracer.py:11: error: Skipping analyzing 'aws_xray_sdk.core': found module but no type hints or library stubs
aws_lambda_powertools/tracing/tracer.py:156: error: Argument "modules" to "patch" of "Tracer" has incompatible type "Optional[List[Any]]"; expected "Optional[Tuple[str]]"
aws_lambda_powertools/tracing/tracer.py:464: error: "Callable[..., Any]" has no attribute "__wrapped__"
aws_lambda_powertools/tracing/tracer.py:476: error: Argument 1 to "wraps" has incompatible type "Optional[Callable[..., Any]]"; expected "Callable[..., Any]"
aws_lambda_powertools/tracing/tracer.py:500: error: Argument 1 to "wraps" has incompatible type "Optional[Callable[..., Any]]"; expected "Callable[..., Any]"
aws_lambda_powertools/tracing/tracer.py:521: error: Argument 1 to "wraps" has incompatible type "Optional[Callable[..., Any]]"; expected "Callable[..., Any]"
aws_lambda_powertools/tracing/tracer.py:541: error: Argument 1 to "wraps" has incompatible type "Optional[Callable[..., Any]]"; expected "Callable[..., Any]"
aws_lambda_powertools/tracing/tracer.py:601: error: Item "None" of "Optional[Any]" has no attribute "put_metadata"
aws_lambda_powertools/metrics/base.py:11: error: Skipping analyzing 'fastjsonschema': found module but no type hints or library stubs
aws_lambda_powertools/metrics/base.py:11: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
aws_lambda_powertools/metrics/base.py:95: error: Cannot determine type of 'metadata_set'
aws_lambda_powertools/metrics/base.py:211: error: Argument 1 to "update" of "dict" has incompatible type "Dict[str, float]"; expected "Mapping[str, Dict[str, object]]"
aws_lambda_powertools/metrics/base.py:311: error: Incompatible return value type (got "Union[str, MetricUnit]", expected "str")
aws_lambda_powertools/utilities/validation/base.py:4: error: Skipping analyzing 'fastjsonschema': found module but no type hints or library stubs
aws_lambda_powertools/utilities/validation/base.py:5: error: Skipping analyzing 'jmespath': found module but no type hints or library stubs
aws_lambda_powertools/utilities/validation/base.py:6: error: Skipping analyzing 'jmespath.exceptions': found module but no type hints or library stubs
aws_lambda_powertools/metrics/metric.py:45: error: Argument 2 of "add_metric" is incompatible with supertype "MetricManager"; supertype defines the argument type as "Union[MetricUnit, str]"
aws_lambda_powertools/metrics/metric.py:45: note: This violates the Liskov substitution principle
aws_lambda_powertools/metrics/metric.py:45: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
aws_lambda_powertools/metrics/metric.py:113: error: Name 'metric_set' already defined on line 108
aws_lambda_powertools/logging/logger.py:122: error: Variable "sys.stdout" is not valid as a type
aws_lambda_powertools/logging/logger.py:122: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
aws_lambda_powertools/logging/logger.py:127: error: Argument 1 to "_get_log_level" of "Logger" has incompatible type "Union[str, int, None]"; expected "Union[str, int]"
aws_lambda_powertools/logging/logger.py:282: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "str")
aws_lambda_powertools/logging/logger.py:301: error: Variable "sys.stdout" is not valid as a type
aws_lambda_powertools/logging/logger.py:301: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
aws_lambda_powertools/middleware_factory/factory.py:118: error: Item "None" of "Optional[Callable[..., Any]]" has no attribute "__qualname__"
aws_lambda_powertools/metrics/metrics.py:69: error: Need type annotation for '_metrics' (hint: "_metrics: Dict[<type>, <type>] = ...")
aws_lambda_powertools/metrics/metrics.py:70: error: Need type annotation for '_dimensions' (hint: "_dimensions: Dict[<type>, <type>] = ...")
aws_lambda_powertools/metrics/metrics.py:71: error: Need type annotation for '_metadata' (hint: "_metadata: Dict[<type>, <type>] = ...")
aws_lambda_powertools/utilities/validation/validator.py:113: error: Argument "data" to "unwrap_event_from_envelope" has incompatible type "Union[Dict[Any, Any], str]"; expected "Dict[Any, Any]"
aws_lambda_powertools/utilities/validation/validator.py:113: error: Argument "jmespath_options" to "unwrap_event_from_envelope" has incompatible type "Optional[Dict[Any, Any]]"; expected "Dict[Any, Any]"
aws_lambda_powertools/utilities/validation/validator.py:117: error: Argument "data" to "validate_data_against_schema" has incompatible type "Union[Dict[Any, Any], str]"; expected "Dict[Any, Any]"
aws_lambda_powertools/utilities/validation/validator.py:202: error: Argument "jmespath_options" to "unwrap_event_from_envelope" has incompatible type "Optional[Dict[Any, Any]]"; expected "Dict[Any, Any]"
aws_lambda_powertools/utilities/validation/validator.py:204: error: Argument "schema" to "validate_data_against_schema" has incompatible type "Optional[Dict[Any, Any]]"; expected "Dict[Any, Any]"
aws_lambda_powertools/utilities/parameters/ssm.py:9: error: Skipping analyzing 'botocore.config': found module but no type hints or library stubs
aws_lambda_powertools/utilities/parameters/ssm.py:147: error: Item "None" of "Optional[Any]" has no attribute "get_parameter"
aws_lambda_powertools/utilities/parameters/ssm.py:171: error: Item "None" of "Optional[Any]" has no attribute "get_paginator"
aws_lambda_powertools/utilities/parameters/secrets.py:9: error: Skipping analyzing 'botocore.config': found module but no type hints or library stubs
aws_lambda_powertools/utilities/parameters/secrets.py:87: error: Item "None" of "Optional[Any]" has no attribute "get_secret_value"
aws_lambda_powertools/utilities/parameters/dynamodb.py:10: error: Skipping analyzing 'botocore.config': found module but no type hints or library stubs
aws_lambda_powertools/utilities/parameters/dynamodb.py:183: error: Item "None" of "Optional[Any]" has no attribute "get_item"
aws_lambda_powertools/utilities/parameters/dynamodb.py:198: error: Argument 1 to "Key" has incompatible type "Optional[str]"; expected "str"
aws_lambda_powertools/utilities/parameters/dynamodb.py:200: error: Item "None" of "Optional[Any]" has no attribute "query"
aws_lambda_powertools/utilities/parameters/dynamodb.py:206: error: Item "None" of "Optional[Any]" has no attribute "query"
aws_lambda_powertools/utilities/batch/base.py:143: error: "None" not callable
aws_lambda_powertools/utilities/batch/base.py:144: error: Item "None" of "Optional[BasePartialProcessor]" has no attribute "process"
aws_lambda_powertools/utilities/batch/sqs.py:10: error: Skipping analyzing 'botocore.config': found module but no type hints or library stubs
aws_lambda_powertools/utilities/batch/sqs.py:72: error: Return value expected

@gmcrocetti
Copy link
Contributor

gmcrocetti commented 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:

  • Create py.typed file;
  • Include in [poetry.tool].include;
  • Solve mypy errors (one issue per sub-package);
  • Add mypy into CI pipeline.

@huonw
Copy link
Contributor Author

huonw commented Dec 7, 2020

I'm mostly new to mypy and assumed type hints were enough.

Yeah, I've been tripped up by that too!

Is adding py.typed into the root of the project all we need to do? If so, it's a no brainier indeed.

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've just run mypy against the code base and seem to have picked up quite a lot of issues - If we need to fix them first before adding py.typed, it might take a week or two.

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
Copy link
Contributor

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.

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 ?

@gmcrocetti gmcrocetti mentioned this issue Dec 8, 2020
4 tasks
@heitorlessa heitorlessa self-assigned this Dec 11, 2020
@heitorlessa
Copy link
Contributor

Understood! Thanks for the quick PR @gmcrocetti - Perhaps it's possible to do a pip install from develop branch using (git+https) then running mypy to 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
Copy link
Contributor Author

huonw commented Dec 14, 2020

Awesome! It seems to mostly work 🎉 I can remove the type: ignore annotations 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@3ac1ae3d28f66095c07c43c6912a4a8cee09a021

It 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:

    # works in mypy
    from aws_lambda_powertools.utilities.typing import LambdaContext
    from aws_lambda_powertools.logging import Logger
    from aws_lambda_powertools.tracing import Tracer
    
    # doesn't work in mypy
    from aws_lambda_powertools import Logger, Tracer 
    # path/to/file.py:123: Module 'aws_lambda_powertools' has no attribute 'Tracer'
    # path/to/file.py:123: Module 'aws_lambda_powertools' has no attribute 'Logger'
  • with @tracer.capture_lambda_handler and @logger.inject_lambda_context (and others, potentially), the final decorated function has the wrong type, rather than Callable[[Dict[str, Any], LambdaContext], Any] or Callable[[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:

    LambdaHandlerCallable = Callable[..., ...] # one of the types above
    
    def decorate(f: Callable[..., ...]) -> LambdaHandlerCallable:
        @tracer.capture_lambda_handler
        @logger.inject_lambda_context
        @functools.wraps(f)
        def wrapper(...):
            ...
    
        return cast(LambdaHandlerCallable, wrapper)
    
    
    @decorate
    def handler_one(...):
        return ...
    
    @decorate
    def handler_two(...):
        return ...

@heitorlessa
Copy link
Contributor

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
Copy link
Contributor Author

huonw commented 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:

from typing import overload, TypeVar, Any, Callable, Union, Optional

_Func = TypeVar("_Func", bound=Callable[..., Any])


class Tracer:
    @overload
    def capture_method(
        self, *, capture_response: bool = True
    ) -> Callable[[_Func], _Func]:
        ...

    @overload
    def capture_method(self, method: _Func) -> _Func:
        ...

    def capture_method(
        self, method: Optional[_Func] = None, *, capture_response: bool = True
    ) -> Union[_Func, Callable[[_Func], _Func]]:
        raise NotImplementedError("original implementation here...")


# demo, the generics ensures the function types are kept accurately:
tracer = Tracer()

@tracer.capture_method
def f(a: int, b: float = 1.23) -> str:
    return ""

reveal_type(f) # 'def (a: builtins.int, b: builtins.float =) -> builtins.str'

@tracer.capture_method(capture_response=False)
def g(a: str, b: None = None) -> int:
    return 0

reveal_type(g) # 'def (a: builtins.str, b: None =) -> builtins.int'

@heitorlessa
Copy link
Contributor

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

@aws-powertools aws-powertools locked and limited conversation to collaborators Mar 12, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
feature-request feature request help wanted Could use a second pair of eyes/hands
Projects
None yet
Development

No branches or pull requests

3 participants