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.

Sign up for GitHub

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

Jump to bottom

Incorrect type narrowing when unpacking union of tuple on an existing variable #12915

Open
nils-van-zuijlen opened this issue May 31, 2022 · 1 comment · May be fixed by #14423 or #14440
Open

Incorrect type narrowing when unpacking union of tuple on an existing variable #12915

nils-van-zuijlen opened this issue May 31, 2022 · 1 comment · May be fixed by #14423 or #14440
Labels
bug mypy got something wrong priority-0-high topic-type-narrowing Conditional type narrowing / binder

Comments

@nils-van-zuijlen
Copy link

nils-van-zuijlen commented May 31, 2022 •
edited

Bug Report

When unpacking a tuple with Union types over existing variables, some of the types disappear.

To Reproduce

import random


def foo() -> tuple[int, None] | tuple[int, str]:
    if random.choice((True, False)):
        return 0, None
    return 5, "thing"


res = None

status, res = foo()
reveal_type(res)  # builtins.str

Expected Behavior

res should be an Optional[str], not a str.

Actual Behavior

The Optional gets lost in the assignment.

Your Environment

  • Mypy version used: 0.950
  • Mypy command-line flags:
  • Mypy configuration options from mypy.ini (and other config files):
  • Python version used: 3.10
  • Operating system and version: Manjaro Linux x86_64 5.15.41-1-MANJARO

This may be related to #9731, but I'm not sure about it.
@nils-van-zuijlen nils-van-zuijlen added the bug mypy got something wrong label May 31, 2022
@JelleZijlstra JelleZijlstra added the topic-type-narrowing Conditional type narrowing / binder label May 31, 2022
@JelleZijlstra
Copy link
Member

Wow, that's pretty bad. It reproduces on all versions in the playground. It happens only with the res = None assignment, so I guess it has something to do with partial types.

tyralla added a commit to tyralla/mypy that referenced this issue Jan 10, 2023
@tyralla
Fix checking multiple assignments based on tuple unpacking involving …
fc29b93 
…partially initialised variables (Fixes python#12915).

This commit introduces two changes:
 * It converts unions of tuples to tuples of unions during multi-assignment checking when all (original) tuples have the same length.  This fixes issue python#12915.
 * It removes the `undefined_rvalue` logic completely.  This logic bothered me because it introduced the necessity to make the mentioned conversion twice (which was not always successful due to reasons I do not fully understand).

Test new case `testDefinePartiallyInitialisedVariableDuringTupleUnpacking` covers issue python#12915.

I had to adjust some other test cases.  In my opinion, the error messages are now more transparent, consistent, and complete.  Removing the `undefined_rvalue` logic also seems to fix one more bug.  In the test cases `testInferenceWithTypeVariableTwiceInReturnType` and `testInferenceWithTypeVariableTwiceInReturnTypeAndMultipleVariables`, there were three assignments for which Mypy did not report errors but, in my opinion, should.

So, for the current test data set, removing the `undefined_rvalue` logic seems to introduce mere benefits than drawbacks (meaning, different error messages than before).  I hope it does not break any Mypy functionalities not covered by the tests.
@tyralla tyralla linked a pull request Jan 10, 2023 that will close this issue
Open
tyralla added a commit to tyralla/mypy that referenced this issue Jan 12, 2023
@tyralla
Fix checking multiple assignments based on tuple unpacking involving …
d7a8c04 
…partially initialised variables (Fixes python#12915).

This proposal is an alternative to python#14423.  Similar to python#14423, the main idea is to convert unions of tuples to tuples of (simplified) unions during multi-assignment checks.  In addition, it extends this idea to other iterable types, which allows removing the `undefined_rvalue` logic and the `no_partial_types` logic.  Hence, the problem reported in python#12915 with partially initialised variables should be fixed for unions that combine, for example, tuples and lists, as well.

Besides the new test case also provided by python#14423 (`testDefinePartiallyInitialisedVariableDuringTupleUnpacking`), this commit also adds the test cases `testUnionUnpackingIncludingListPackingSameItemTypes`, `testUnionUnpackingIncludingListPackingDifferentItemTypes`, and `testUnionUnpackingIncludingListPackingForVariousItemTypes`.
@tyralla tyralla linked a pull request Jan 12, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug mypy got something wrong priority-0-high topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Milestone
No milestone
3 participants
@JelleZijlstra @hauntsaninja @nils-van-zuijlen