-
-
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
TypedDict 'in' narrowing w/o @final #15698
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Positive effect in Rapptz/discord.py: |
@hauntsaninja since you approved #13838, can take a look? |
d_final: DFinal | ||
|
||
if 'spam' in d_final: | ||
spam = 'ham' # E: Statement is unreachable |
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.
Is this test really useless? E.g. this seems useful and I'm not sure where else it is tested
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.
Good point, brought it back in a68b87a (and also removed more @final
s).
This comment has been minimized.
This comment has been minimized.
@hauntsaninja another review? |
Diff from mypy_primer, showing the effect of this PR on open source code: discord.py (https://github.com/Rapptz/discord.py)
- discord/audit_logs.py:251: error: Argument "allow_list" to "AutoModTrigger" has incompatible type "object"; expected "list[str] | None" [arg-type]
- discord/audit_logs.py:257: error: Argument "allow_list" to "AutoModTrigger" has incompatible type "object"; expected "list[str] | None" [arg-type]
- discord/audit_logs.py:258: error: Argument "regex_patterns" to "AutoModTrigger" has incompatible type "object"; expected "list[str] | None" [arg-type]
|
This issue recently came up on another forum, and it seems the fix is stuck in limbo. The diff seems fairly innocuous, the build/tests pass, and the discussion elsewhere seemed to conclude that this is the correct fix. Further, pyright merged the equivalent fix. Is it possible for this to be revisited? |
@hauntsaninja Can we revisit this? |
I think it would be good to try and get whatever behaviour is desired here into the typing spec. Would you or gandhis1 be willing to follow the steps here? https://github.com/python/typing-council#decisions |
We shouldn't require
T1
andT2
to be final, to have'foo' in Union[TD1, TD2]
narrowing take place. We previously (#13838) thought we should, but @erictraut made a good point that TypedDicts are structured types (like interfaces in TypeScript) and we shouldn't consider class hierarchy when type-matching but rather just the structure.Fixes #15697.