Fix #1855: Multiassign from Union (take 2) #2219
Conversation
It's just getting uglier :| Using silenced period for the binder.
It's just getting uglier :| Using silenced period for the binder.
| lr_pairs.append((star_lv.expr, rv_list)) | ||
| lr_pairs.extend(zip(right_lvs, right_rvs)) | ||
| for lv, rv in lr_pairs: | ||
| self.check_assignment(lv, rv, infer_lvalue_type) |
There was a problem hiding this comment.
Nothing new in this function. It's only extracted, and the comment is slightly more elaborated.
|
@JukkaL I think I have addressed most of your comments from the original PR. Tell me if I'm missing anything. Also I am not confident with my "silence the binder" solution. I think it's ugly. |
Conflicts: mypy/binder.py
|
@elazarg This has been languishing for a month now. Do you still want to work on this? Despite what GitHub says, it actually requires help rebasing (I tried and failed -- maybe because there are so many small local commits). Maybe you can restart this work by separating it in two parts, one that just moves code around as you wish without any changes in behavior (as proved by passing all tests without any test editing) and then a second one that actually makes the small(ish) changes needed to fix the issue. |
|
I will make it merge-able after #2238 is merged, since both deal with the binder. |
|
Since @2238 is now merged, can you update this too? |
|
I should, but it's pretty invasive and old so it will be tricky. And perhaps I should take a fresh look at the whole thing. |
|
@elazarg Are you going to continue working on this? I think this is an important PR and I will be glad if it is finished. |
|
I tried, but the solutions I have in mind are too fundamental and will introduce too much churn. Sorry. |
Fixes #3859 Fixes #3240 Fixes #1855 Fixes #1575 This is a simple fix of various bugs and a crash based on the idea originally proposed in #2219. The idea is to check assignment for every item in a union. However, in contrast to #2219, I think it will be more expected/consistent to construct a union of the resulting types instead of a join, for example: ``` x: Union[int, str] x1 = x reveal_type(x1) # Revealed type is 'Union[int, str]' y: Union[Tuple[int], Tuple[str]] (y1,) = y reveal_type(y1) # Revealed type is 'Union[int, str]' ``` @elazarg did the initial work on this.
Reopening #2154. Fix #1855: the union case was simply not handled (Probably fixes #1575 too).
I moved some code around so the cases are syntactically together. It makes the diff harder to read; some functions are easier to read without the diff. Most of the actual content is
check_multi_assign_from_union()The awkward part is the treatment of binding: I use a context manager to make the binding return a list of actions instead of doing the actual actions.