MousaZeidBaker / aws-lambda-typing

Python type hints for AWS Lambda
MIT License
106 stars 19 forks source link

Total vs optional/NotRequired #96

Open PierreSnell opened 10 months ago

PierreSnell commented 10 months ago

First of all, thanks a lot for the typing! Really helpful.

Sorry for the (maybe) dumb question but could the TypeDict have optional keys instead of a total=False?

As an S3Object, could always have some key but the value could be null ?

So

class SomeType(TypedDict, total=False): 
    myAlwaysPresentKey: str

would become

class SomeType(TypedDict): 
    myAlwaysPresentKey: str | None # Or Optional[str] for python < 3.10

else, it could also be NotRequired which indicates to python that the key can be not here (without total=False which makes all the keys NotRequired)

class SomeType(TypedDict): 
    myAlwaysPresentKey: NotRequired[str] # If key can be optional but when it's here it's sure it has a value
# Or 
class SomeType(TypedDict): 
    myAlwaysPresentKey: NotRequired[str | None] # If key and value can be None

That would make code like below compliant with type checkers :

# Could not access the item in TypedDict
# "s3" is not a required key in "S3EventRecord", so access may result in a runtime exception
s3_object: S3 = event["Records"][0]["s3"]

Thanks in advance and sorry if indeed all keys are optional then my question doesn't make sense.

MousaZeidBaker commented 9 months ago

Great to hear you're pleased with the project!

Let's start by clarifying the concept of Optional syntax. As per the documentation

Optional[int]  # means int|None, not potentially-missing!

the above indicates that the key is required while the value can be either int or none. Not the same thing as NotRequired.

At the time of developing this project, the only option to define non required keys was to use total=False which isn't really convenient as this makes all keys in the dictionary non required. I can see that they have introduced NotRequired syntax in Python <3.11 which can be used on individual keys.

I'm open to adopt the new syntax, but we'll need to perform some detective work to distinguish between keys that are non-required and those that are required but can be set to none. This task isn't straightforward due AWS documentation isn't always clear on this.

PierreSnell commented 9 months ago

Hello @MousaZeidBaker,

Thanks a lot for the answer.

I agree that Optional is not the best solution, it was only to propose multiple approaches, both compliant to type-checkers, but I also prefer the NotRequired approach.

For sure that's some long and difficult work (because of aws, and docs.....), and the intention is to give more of an idea :)

I am glad it pleases you, you could close the issue if you want as I guess it will be part of future small improvements, or keep it open to keep track.

Thanks again, Have a great day