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 type inference for tuples in iterable context #13406

Merged
merged 2 commits into from
Aug 14, 2022

Conversation

ilevkivskyi
Copy link
Member

It is a minor change to fix the example in python/typeshed#7904

@ilevkivskyi
Copy link
Member Author

cc @JukkaL

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

Well, this shows how fragile is type inference that has neither backtracking nor full-expression scope. Here pytest uses a type like Iterable[Union[Tuple[str, Dict[str, int], str], Sequence[object]]] so after my first commit, a tuple of three item tuples, where one of items has second item unlucky to be a dict, there is an error, although such three item tuple would perfectly match Sequence[object].

I tried a fix (which btw makes things more consistent to be fair). If this will still cause problems as well, we can also try simplifying unions before providing it as a context for tuple.

@ilevkivskyi
Copy link
Member Author

Actually, I am almost sure simplifying the union is the right thing to do. What if context is a union of two identical tuple types? in this case we currently give up and don't provide a context at all.

@ilevkivskyi
Copy link
Member Author

OTOH if original union was like Union[Tuple[int, int], object], we will destroy the tuple by simplifying. Maybe we can try both original and simplified union and choose the one that has exactly one tuple-like type.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@ilevkivskyi
Copy link
Member Author

Hm, OK, there is no effect in mypy_primer so maybe let's just not touch it. If we will discover some more issues later, we can try what I mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant