Add intelligent indexing of tuples, NamedTuples, and TypedDict#6124
Merged
Michael0x2a merged 3 commits intopython:masterfrom Jan 4, 2019
Merged
Conversation
This pull request adds a preliminary implementation of intelligent indexing of tuples, NamedTuples, and TypedDicts. It uses the first approach we discussed earlier: modifying the existing plugins and special-casing code to also check if the expression has a Literal[...] type. Once I'm finished with the baseline literal types implementation, I'll look into circling back and seeing how viable the second approach is (writing some sort of plugin that replaces the signatures of methods like `.__getitem__` or `.get()` with overloads that use the appropriate literal types).
Closed
msullivan
approved these changes
Jan 4, 2019
Collaborator
msullivan
left a comment
There was a problem hiding this comment.
Looks good to me! The logic is sort of notionally more complicated but the code flow is arguably cleaner so it seems like a positive anyways.
| item_name = typ.value | ||
| else: | ||
| self.msg.typeddict_key_must_be_string_literal(td_type, index) | ||
| return AnyType(TypeOfAny.from_error) |
Collaborator
There was a problem hiding this comment.
Maybe not really a question for this diff, but is there a reason for this to not live in the typeddict plugin as a __getitem__ hook?
There is maybe a more relevant question of: could this just use try_getting_str_literal, the answer to which would be more obvious if this lived in the plugin.
Collaborator
Author
There was a problem hiding this comment.
I agree -- I think we should move both this and tuple.__getitem__ out into a plugin, but it felt like it'd be out-of-scope for this PR.
42 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds a preliminary implementation of intelligent indexing of tuples, NamedTuples, and TypedDicts. It uses the first approach we discussed earlier: modifying the existing plugins and special-casing code to also check if the expression has a Literal[...] type.
Once I'm finished with the baseline literal types implementation, I'll look into circling back and seeing how viable the second approach is (writing some sort of plugin that replaces the signatures of methods like
.__getitem__or.get()with overloads that use the appropriate literal types).