Be consistent in type hinting named parameters

[giovannt0]

Ideally we would have def foo(bar: Optional[Type] = None) -> ReturnType for named parameters which default to None instead of def foo(bar: Type = None) -> ReturnType. As of now we have both across the codebase.

Note: we want to use Optional where it makes sense from mypy POV. def foo(bar: Optional[str] = 'default-string') -> ReturnType does not make sense because Optional essentially means Union[type, None]. In this case this would mean Union[str, None] and unless we're specifically allowing / expecting that bar may be None, Optional shouldn't be used here.

[hacktobeer]

If a parameter can have a None/'' value it's Optional, else it's not Optional, correct? Named parameters are Optional.

[giovannt0]

Yes yes, mypy accepts both forms of type hinting but I find it cleaner to explicitly use the Optional keyword in the method's signature (and most methods are currently using it, so it's more about using it everywhere or not use it at all).

[hacktobeer]

but we should only use Optional when the parameter is optional, right?

On Fri, Jun 26, 2020 at 9:17 AM Theo notifications@github.com wrote:

Yes yes, mypy accepts both forms of type hinting but I find it cleaner to explicitly use the Optional keyword in the method's signature (and most methods are currently using it, so it's more about using it everywhere or not use it at all).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/google/cloud-forensics-utils/issues/171#issuecomment-650020234, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABP5D4EZ3WEUTIVCXIM2KVTRYRDQLANCNFSM4OJBLJZQ .

[giovannt0]

Yes! Anything of the form func(param = value) would be type hinted as func(param: Optional[Type] = value) (instead of func(param: Type = value))

[tomchop]

If a parameter can have a None/'' value it's Optional, else it's not Optional, correct? Named parameters are Optional.

to clarify: as far as mypy is concerned, Optional parameters should be used only for keywords arguments that have no default value (i.e. that default to None). Any keyword argument that declares a default value is implicitly optional for mypy, and its type defaults to the type of the declared default value.

TLDR: We want to use Optional only when the parameter is optional and we expect that parameter to be None if it is not explicitly passed by the caller.