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

Add checks for overlapping TupleTypes #4238

Merged
merged 3 commits into from Nov 24, 2017

Conversation

Projects
None yet
2 participants
@elazarg
Contributor

elazarg commented Nov 13, 2017

Fix #4237.

This is not complete or bulletproof, but it's an improvement.

I don't see where should there be a special case for namedtuple; maybe there shouldn't.

@JukkaL

Thanks for fixing this! Looks good, just one question.

Show outdated Hide outdated mypy/meet.py Outdated

elazarg added some commits Nov 15, 2017

Move unions before tuples
Since unions can overlap with basically everything, checking for them first
gives more confidence in later checks, such as tuple types and type types.
@elazarg

This comment has been minimized.

Show comment
Hide comment
@elazarg

elazarg Nov 15, 2017

Contributor

Added support for Tuple[A, ...].
I also fixed the use of fallback - I believe that comparing fallbacks is incorrect.

@JukkaL the logic is quite different now, so it needs to be re-reviewed... Sorry about that :\

There are many tests that can be added - involving combinations of tuples, namedtuple, varargs, instances and unions; but I prefer to do it in at a different time.

Contributor

elazarg commented Nov 15, 2017

Added support for Tuple[A, ...].
I also fixed the use of fallback - I believe that comparing fallbacks is incorrect.

@JukkaL the logic is quite different now, so it needs to be re-reviewed... Sorry about that :\

There are many tests that can be added - involving combinations of tuples, namedtuple, varargs, instances and unions; but I prefer to do it in at a different time.

@JukkaL

JukkaL approved these changes Nov 24, 2017

LGTM

@JukkaL JukkaL merged commit e180c7c into python:master Nov 24, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment