Conversation
change type hints in ExceptionMiddleware
aminalaee
force-pushed
the
fix-typehint-of-exception-handlers
branch
from
b6cf47e to
42902fb
Compare
| app: ASGIApp, | ||
| handlers: typing.Mapping[ | ||
| typing.Any, typing.Callable[[Request, Exception], Response] | ||
| ] = None, |
There was a problem hiding this comment.
It's quite a complicated type, should we move it to starlette.types? Like ExceptionHandler type
There was a problem hiding this comment.
The current types on types.py are going to be removed at some point: #1217
That file will only contain this type at some point. Is that a problem? 🤔
There was a problem hiding this comment.
Didn't notice the Issue,
This mapping type just seems a bit complicated to me to be copied multiple times
There was a problem hiding this comment.
One drawback I noticed in VScode, haven't really checked and there might be some workarounds, is that they don't show function signatures when using the aliases as you mention. Not sure if it happens on PyCharm and other tools.
|
Speaking about |
|
@aminalaee @k4black FYI, I've opened a PR that improves on the typing added in this PR: #1456 |
4 participants
Closes #1142 and replaces #1139.
Changes
exception_handlerstype hints:I also changed the callable type hints, I think it makes sense to be more explicit, this will catch user-defined callable mismatches (?).
As mentioned in #1139 we can't use
typing.Mapping[typing.Union[int, ..], ...]since dict keys are not covariant.I think we could use.typing.TypedDictbut only after 3.8