-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
Proposal: don't simplify unions in expand_type() #14178
Proposal: don't simplify unions in expand_type() #14178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this! I think that we've been simplifying unions too aggressively. Not simplifying unions "should not" affect semantics, except for the details of revealed types (and bugs related to union types).
Accepting on the assumption that mypy_primer doesn't show meaningful regressions.
This comment has been minimized.
This comment has been minimized.
The one in |
Diff from mypy_primer, showing the effect of this PR on open source code: operator (https://github.com/canonical/operator)
- ops/testing.py:906: error: Argument 2 to "get" of "dict" has incompatible type "None"; expected "Mapping[str, str]" [arg-type]
+ ops/testing.py:906: error: Argument 2 to "get" of "dict" has incompatible type "None"; expected "Union[Dict[str, str], Mapping[str, str]]" [arg-type]
prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/utilities/asyncutils.py:205: error: Argument 1 to "is_async_fn" has incompatible type "T"; expected "Callable[[VarArg(<nothing>), KwArg(<nothing>)], Awaitable[<nothing>]]" [arg-type]
+ src/prefect/utilities/asyncutils.py:205: error: Argument 1 to "is_async_fn" has incompatible type "T"; expected "Union[Callable[[VarArg(<nothing>), KwArg(<nothing>)], <nothing>], Callable[[VarArg(<nothing>), KwArg(<nothing>)], Awaitable[<nothing>]]]" [arg-type]
discord.py (https://github.com/Rapptz/discord.py)
- discord/automod.py:284: error: Argument "data" to "from_data" of "AutoModTrigger" has incompatible type "object"; expected "Optional[Empty]" [arg-type]
+ discord/automod.py:284: error: Argument "data" to "from_data" of "AutoModTrigger" has incompatible type "object"; expected "Union[_AutoModerationTriggerMetadataKeyword, _AutoModerationTriggerMetadataKeywordPreset, _AutoModerationTriggerMetadataMentionLimit, Empty, None]" [arg-type]
|
OK, now |
Fixes #6730
Currently
expand_type()
is inherently recursive, going throughexpand_type
->make_simplified_union
->is_proper_subtype
->map_instance_to_supertype
->expand_type
. TBH I never liked this, so I propose that we don't do this. One one hand, this is a significant change in semantics, but on the other hand:To make transition smoother, I propose to make trivial simplifications, like removing
<nothing>
(andNone
without strict optional), removing everything else if there is anobject
type, and remove strict duplicates. Notably, with these few things all existing tests pass (and even without it, only half a dozen tests fail onreveal_type()
).