8000 Accept exception_handlers as Mapping by antonagestam · Pull Request #1139 · encode/starlette · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Accept exception_handlers as Mapping #1139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Accept exception_handlers as Mapping #1139

wants to merge 4 commits into from

Conversation

antonagestam
Copy link

This change does two things: the first is that it communicates to the caller that the passed dict isn't going to be mutated. The second is that it allows the caller to pass any Mapping without first casting it to a dict, which is very redundant since that already happens inside __init__.

@Kludex

This comment has been minimized.

Sign in to view
@natanlao natanlao mentioned this pull request Feb 18, 2021
Closed
2 tasks
@JayH5 JayH5 added the typing Type annotations or mypy issues label Mar 11, 2021
@JayH5
Copy link
Member
JayH5 commented Apr 12, 2021

I do think Mapping is the right type here, but the fact that the key type is a Union of two types means that #1142 is still an issue. Mappings and dicts are not covariant in their key types, so really the best we can do is Mapping[Any, Callable]

@aminalaee
Copy link
Member

@antonagestam Will you update this?

aminalaee
aminalaee requested changes Dec 13, 2021
View reviewed changes
@aminalaee aminalaee mentioned this pull request Dec 14, 2021
Merged
@antonagestam
Copy link
Author

I'll try to get to addressing this soon.

@antonagestam
Copy link
Author

Updated to use a contravariant typevar, which makes this also close #1142.

@Kludex
Copy link
Member
Kludex commented Jan 30, 2022

This was closed by #1360.

Thanks for the PR @antonagestam ! :)

@Kludex Kludex closed this Jan 30, 2022
@antonagestam
Copy link
Author

@Kludex Oh, note that I updated this PR to solve the problem without introducing Any for the key type, so I still think there's value in integrating some of that work.

@Kludex Kludex reopened this Jan 30, 2022
@Kludex
Copy link
Member
Kludex commented Jan 30, 2022

Cool! I've opened again! :)

@antonagestam antonagestam mentioned this pull request Jan 30, 2022
Closed
@antonagestam
Copy link
Author

@Kludex I opened #1456 to supersede this one.

@antonagestam antonagestam deleted the patch-1 branch January 30, 2022 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aminalaee aminalaee Awaiting requested review from aminalaee

Assignees
No one assigned
Labels
typing Type annotations or mypy issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@antonagestam @Kludex @JayH5 @aminalaee
0