py(types): Add type annotations to email utils. #29854
Conversation
mgaeta
force-pushed
the
feat/api-2029-webhook-task-10
branch
from
b61c9ff to
38c974a
Compare
|
|
||
| def __init__(self, namespace, type_handlers): | ||
| def __init__( | ||
| self, namespace: str, type_handlers: Mapping[type[Model], Callable[[M], Any]] |
There was a problem hiding this comment.
Isn't the return type of each handler at least Iterable[Any]? I think based on the implementation below it might even be Iterable[str]?
| to: list[str] | None = None, | ||
| cc: Iterable[str] | None = None, |
There was a problem hiding this comment.
Why list for some and Iterable for others?
| fmt = options.get("system.logging-format") | ||
| messages = self.get_built_messages(to, cc=cc, bcc=bcc) | ||
| extra = {"message_type": self.type} | ||
| extra: MutableMapping[str, str | tuple[str]] = {"message_type": self.type} |
There was a problem hiding this comment.
I skimmed around a bit and it looks like the value here should just be str, I didn't see where it might be tuple[str]?
There was a problem hiding this comment.
There's a sneaky tuple on line 244:
extra["message_to"] = (self.format_to(message.to),)
| def get_built_messages( | ||
| self, | ||
| to: list[str] | None = None, | ||
| cc: Iterable[str] | None = None, |
There was a problem hiding this comment.
Why is to a list but cc and bcc are Iterable?
There was a problem hiding this comment.
Good question let me see if i can get it to match.
|
We're not supposed to change functionality in typing PRs but I fixed up the regex function to address your comments. |
Extracting types from #28879.
One tricky thing I'm doing here is turning a
Tuple[Optional[str], Optional[str]]into aOptional[Tuple[str, str]]which required a small code change in a conditional.