Skip to content
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

Support indexing unions containing tuples #6475

Merged
merged 2 commits into from Feb 26, 2019

Conversation

Projects
None yet
2 participants
@ilevkivskyi
Copy link
Collaborator

commented Feb 24, 2019

Fixes #4286
Fixes #2509

The fix is straightforward, map the indexing through union types. I decided to use the existing error message mentioning the full type. IMO Value of type "Union[int, List[int]]" is not indexable is better than Value of type "int" is not idexable.

@Michael0x2a
Copy link
Collaborator

left a comment

I left one small question, but apart from that LGTM!

"""
index = e.index
if isinstance(left_type, UnionType):
return UnionType.make_simplified_union([self.visit_index_with_type(typ, e, left_type)

This comment has been minimized.

Copy link
@Michael0x2a

Michael0x2a Feb 24, 2019

Collaborator

Just to check, should we pass original_type here as well?

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Feb 26, 2019

Author Collaborator

Normally, there should be no nested unions because they are flattened in the constructor, but I agree that it would be more robust to pass it also here.

@ilevkivskyi ilevkivskyi merged commit 69eaf88 into python:master Feb 26, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:union-ops branch Feb 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.