Skip to content

Accept exception_handlers as Mapping#1139

Closed
antonagestam wants to merge 4 commits intoKludex:masterfrom
antonagestam:patch-1
Closed

Accept exception_handlers as Mapping#1139
antonagestam wants to merge 4 commits intoKludex:masterfrom
antonagestam:patch-1

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.

@JayH5 JayH5 added the typing Type annotations or mypy issues label Mar 11, 2021
@JayH5
Copy link
Contributor

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
Contributor

@antonagestam Will you update this?

@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
Owner

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
Owner

Kludex commented Jan 30, 2022

Cool! I've opened again! :)

@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

Labels

typing Type annotations or mypy issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants