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

Fix meet for tuple with instance #5641

Merged
merged 3 commits into from Sep 19, 2018

Conversation

Projects
None yet
2 participants
@ilevkivskyi
Copy link
Collaborator

commented Sep 19, 2018

Fixes #5640

Since generic tuple types are not supported, map_instance_to_supertype is not needed here. This solution doesn't work for general multiple inheritance, for example the test case will not work with if isinstance(o, BaseTuple), but this is exactly the same as for e.g. two instance types.

@JukkaL
Copy link
Collaborator

left a comment

Looks good, I only have one comment.

return t.copy_modified(items=[meet_types(it, self.s.args[0]) for it in t.items])
elif is_proper_subtype(t, self.s):
# A named tuple that inherits from a normal class
return t
elif (isinstance(self.s, Instance) and t.fallback.type == self.s.type):

This comment has been minimized.

Copy link
@JukkaL

JukkaL Sep 19, 2018

Collaborator

It looks like the condition can never be true. Either move this to the above elif block or remove?

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Sep 19, 2018

Author Collaborator

Yes, good catch, I think this is not needed anymore.

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Sep 19, 2018

Author Collaborator

OK, fixed now!

Ivan Levkivskyi

@ilevkivskyi ilevkivskyi merged commit a89aec5 into python:master Sep 19, 2018

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:tuple-regression branch Sep 19, 2018

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.