Typing fixes in config.py #307
Conversation
| @@ -32,7 +32,7 @@ def getenv(env_var: str, default: T) -> Union[str, T]:... | |||
| @overload | |||
| def getenv(env_var: str) -> str:... | |||
|
|
|||
| def getenv(env_var: str, default: T = NOT_PROVIDED) -> Union[str, T]: | |||
| def getenv(env_var: str, default = NOT_PROVIDED) -> str: | |||
There was a problem hiding this comment.
This is inconsistent/not how @overload works. @overloaded function signatures narrow down the return type, given a particular choice of argument types. Annotations on the function implementation itself (without @overload) should be a strict superset of the @overloaded ones. Here, str is a subset of Union[str, T].
There was a problem hiding this comment.
could we make 1 function always return 1 type (str in this case) and use a different funcition name for the other types? or what is the benefit of the overload / generics type?
There was a problem hiding this comment.
@MatthewBaggins originally it was default: T = NOT_PROVIDED, but this raised typing errors saying NOT_PROVIDED was type str, not T. This change fixes that error, with no other errors raised anywhere else in the file. What do you think needs to be different?
There was a problem hiding this comment.
@MatthewBaggins I'm going to go ahead and merge, let me know if you have an alternative way to do it that doesn't raise errors.
ProducerMatt
force-pushed
the
typing-config
branch
from
885c553
to
3a765c4
Compare
No description provided.