Skip to content
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

Incorrect type annotation for parse function #802

Closed
Tracked by #1009
DanLipsitt opened this issue Nov 4, 2021 · 5 comments
Closed
Tracked by #1009

Incorrect type annotation for parse function #802

DanLipsitt opened this issue Nov 4, 2021 · 5 comments
Labels
bug Something isn't working p2

Comments

@DanLipsitt
Copy link

The output type specified for aws_lambda_powertools.utilities.parser.parse does not always match the documented or actual output. The output is often a collection but the specified type is a single object.

Expected Behavior

The return type for parse should match the documented output types for each envelope.

Current Behavior

The return type for parse is Model (the type of the model parameter's input) even though the returned value may be a collection with a more complex type such as List[Dict[str, Optional[Model]]].

Possible Solution

There may be a way to solve this using mypy generics but I can't think of how. The return type could be specified manually as something like the following Union of all the possibilities:

Union[Model, List[Optional[Model]], List[Dict[str, Optional[Model]]]]

This would have to be updated as envelopes are added, and it wouldn't cover custom envelopes.

Alternatively, removing the output type annotation from the toplevel parse might allow the type of the underlying envelope's parse function to bubble up.

Steps to Reproduce

from aws_lambda_powertools.utilities.parser import parse, envelopes, BaseModel


class Item(BaseModel):
    pass


event: dict = {"Records": []}
result: list[Item] = parse(event=event, model=Item, envelope=envelopes.SnsEnvelope)
print(type(result))

Output:

<class 'list'>

Mypy output:

parser_type_example.py:9: error: Incompatible types in assignment (expression has type "Item", variable has type "List[Item]")
Found 1 error in 1 file (checked 1 source file)

Environment

  • Powertools version used:
    1.21.1
  • Packaging format (Layers, PyPi):
    Pypi
  • AWS Lambda function runtime:
    n/a
  • Debugging logs
    n/a
@DanLipsitt DanLipsitt added bug Something isn't working triage Pending triage from maintainers labels Nov 4, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Nov 4, 2021

Thanks for opening your first issue here! We'll come back to you as soon as we can.

@heitorlessa
Copy link
Contributor

Thanks a lot @DanLipsitt for flagging it.

I confess this has been a flaky area in Parser - we had generics and it half-worked - perhaps an @overload specifically when envelope is set might work.

@heitorlessa heitorlessa added area/parser p2 and removed triage Pending triage from maintainers labels Nov 12, 2021
@heitorlessa
Copy link
Contributor

Looking into this now to see if a quick fix I have in mind can solve, otherwise we'll tackle it for the release after next. Thanks a lot for the sample to reproduce as it helps a ton!

@heitorlessa
Copy link
Contributor

@DanLipsitt got it working - Could you please check this PR to double check this is what you'd expect?

#885

If you can confirm by EOD we can make it into the next release, otherwise, I'll pause this for a while as everything I've tried didn't work (Unions, TypeVar + Type variables, etc.)

Thank you!

@heitorlessa heitorlessa added the pending-release Fix or implementation already in dev waiting to be released label Dec 17, 2021
@heitorlessa
Copy link
Contributor

Now released as part of 1.23 version

@heitorlessa heitorlessa removed the pending-release Fix or implementation already in dev waiting to be released label Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2
Projects
None yet
Development

No branches or pull requests

2 participants