[conflict] Fix #1855: Multiassign from Union #2154

elazarg · 2016-09-18T22:45:24Z

Fix #1855: the union case was simply not handled.

I moved some code around so the cases are syntactically together.

I use join instead of union, but it's always finite and most of the time very short, so using unions seems feasible.

(Probably fixes #1575 too)

elazarg · 2016-09-19T02:51:58Z

mypy/checker.py

+            if inferred:
+                self.set_inferred_type(inferred, lv, union)
+            else:
+                self.store_type(lv, union)


The binding of the joined type is "retrofitted". I am not sure this is the best way to do it - maybe inferred vars should simply accumulate through joins?

Retrofitting like this may not be a problem, but I suspect it might be. Accumulating through joins might not be any better. The most correct approach would be to infer the entire type for an lvalue before setting it, so that intermediate values can't leak. I don't know, but it may be possible that the intermediate lvalue types leak to the rvalue in some weird corner cases.

I'd recommend not worrying about this too much, but I'd suggest adding a TODO comment about the potential issues.

gvanrossum · 2016-09-19T16:27:53Z

I think this is great but I don't know that code very well so I hope @JukkaL can review. Have you looked at #2128 yet?

elazarg · 2016-09-19T16:35:12Z

Only very briefly (perhaps I will get to it shortly). From a first glance it seems unrelated. I don't have the right overloads for int+float in the builtin fixture, but perhaps I can manage without it.

gvanrossum · 2016-09-19T16:44:19Z

I don't have the right overloads for int+float in the builtin fixture, but perhaps I can manage without it.

Hm, there's automatic promotion from int->float as needed in mypy, so the overloads shouldn't be needed; also the __radd__ methods might help (or might make things worse, see. e.g. #2129 and the issues referenced from there).

elazarg · 2016-09-19T16:49:06Z

i = None  # type: int
f = None  # type: float
i + f

I get error: Unsupported operand types for + ("int" and "float")

JukkaL · 2016-09-19T16:55:45Z

@elazarg You'll likely need overloads in your builtins fixture for that to work in a test case.

elazarg · 2016-09-19T16:57:30Z

Thanks, I will work on that. Again, it is unrelated to this PR.

JukkaL · 2016-09-21T11:56:14Z

mypy/checker.py

+    def check_multi_assign(self, lvalues: List[Expression],
+                           rvalue: Expression, rvalue_type: Type,
+                           context: Context, infer_lvalue_type: bool = True,
+                           undefined_rvalue = False) -> None:


No annotation for undefined_rvalue.

JukkaL · 2016-09-21T12:15:12Z

mypy/checker.py

-                                          infer_lvalue_type)
-        else:
-            self.msg.type_not_iterable(rvalue_type, context)
+    def type_is_iterable(self, rvalue_type: Type) -> bool:


I'd rather not rename the argument to rvalue_type, as this method works for any types, even if it's only used for rvalues right now

Oops that's weird. I have no idea what was I thinking.

JukkaL · 2016-09-21T12:17:03Z

test-data/unit/check-unions.test

+from typing import Union, Tuple, Any
+
+a = None  # type: Tuple[int]
+(a1,) = a


Reveal type of a1.

JukkaL · 2016-09-21T12:17:14Z

test-data/unit/check-unions.test

+(a1,) = a
+
+b = None  # type: Union[Tuple[int], Tuple[float]]
+(b1,) = b


Reveal type of b1.

JukkaL · 2016-09-21T12:18:08Z

test-data/unit/check-unions.test

+from typing import Union, Tuple, Any
+
+d = None  # type: Union[Any, Tuple[float, float]]
+(d1, d2) = d


Reveal types of d1 and d2.

JukkaL · 2016-09-21T12:20:53Z

test-data/unit/check-unions.test

+(d1, d2) = d
+
+e = None  # type: Union[Any, Tuple[float, float], int]
+(e1, e2) = e  # E: 'builtins.int' object is not iterable


Here are additional tests that would be nice to have:

Lvalue items already have declared types, and the union rvalue has either a compatible or non-compatible type.

In an lvalue like x, y only x has declared type, and the type of y will be inferred.

Lvalue item is of form x[i] or x.y with a union rvalue.

Lvalue is like x, *y.

JukkaL · 2016-09-21T13:00:44Z

mypy/checker.py

+            if inferred:
+                self.set_inferred_type(inferred, lv, union)
+            else:
+                self.store_type(lv, union)


Retrofitting like this may not be a problem, but I suspect it might be. Accumulating through joins might not be any better. The most correct approach would be to infer the entire type for an lvalue before setting it, so that intermediate values can't leak. I don't know, but it may be possible that the intermediate lvalue types leak to the rvalue in some weird corner cases.

I'd recommend not worrying about this too much, but I'd suggest adding a TODO comment about the potential issues.

JukkaL · 2016-09-21T13:08:45Z

mypy/checker.py

+            _1, _2, inferred = self.check_lvalue(lv)
+            union = join_type_list(ut)
+            if inferred:
+                self.set_inferred_type(inferred, lv, union)


You'll probably need to put the new type to the binder (maybe self.binder.assign_type(...)). Also add a test case for this. Example where this may make a difference:

class A: pass class B(A): pass class C(A): pass x = None # type: object a = [] # type: Union[List[B], List[C]] x, y = a # now type of x should be A x = 1 # should be ok # now type of x should be int

The assignment to a is not accepted by mypy. I opened #2164

Simple assign_type did not work; I needed something more intrusive than that. I don't like this solution, to be honest. I want to ask the type to split itself, without side effects, but that's not how things works.

When mypy is run with `--show-column-numbers`, error messages include columns. Column numbers are 0-based. Fixes python#1216.

It's just getting uglier :| Using silenced period for the binder.

* Fix python#2184.

…2200) * Fix strict none errors in the test subpackage. This shows how to deal with python#2199 for `p[i].arg`. * Run mypy over itself in --strict-optional mode. Add mypy.ini to suppress most errors (for now).

Also set options.show_traceback in all test modules I could find. These flags used to be implemented as globals in errors.py that had to be set by calling set_show_tb() or set_drop_into_pdb() (and setting the corresponding Options attributes had no effect). Now they're just regular options. I had to adjust some plumbing to give report_internal_error() access to the options but it was easier than I thought.

Fixes python#2212. All that's left is some calls to in_checked_function(), which replaces the old typing_mode_full() and typing_mode_none() (its negation after the elimination of typing_mode_weak()).

It's just getting uglier :| Using silenced period for the binder.

Conflicts: mypy/binder.py mypy/checker.py

elazarg · 2016-10-05T15:22:32Z

(Again sorry about the noise)

elazarg · 2016-10-05T15:24:33Z

So I used rebase instead of merge, which made the diff impossible... How can I fix it? Close and reopen the PR?

gvanrossum · 2016-10-05T15:35:09Z

So many users, so many git workflows... Just close and create a new PR.

elazarg · 2016-10-05T15:36:03Z

Reopened as #2219

elazarg added 3 commits September 19, 2016 01:30

handle union, add test

7e043d0

kill blank

9a50d73

more tests

5c4d86e

elazarg changed the title ~~Multiassign from Union~~ Fix #1855: Multiassign from Union Sep 18, 2016

elazarg changed the title ~~Fix #1855: Multiassign from Union~~ Fix #1855: Multiassign from Union (in progress) Sep 19, 2016

handle binding

60cfbbb

elazarg changed the title ~~Fix #1855: Multiassign from Union (in progress)~~ Fix #1855: Multiassign from Union Sep 19, 2016

elazarg added 2 commits September 19, 2016 05:40

try to minimize visual difference

5e9e0f2

(cont.)

71f8475

elazarg commented Sep 19, 2016

View reviewed changes

JukkaL reviewed Sep 21, 2016

View reviewed changes

elazarg mentioned this pull request Sep 21, 2016

Error initializing Union of Lists with an empty list #2164

Closed

elazarg and others added 12 commits September 22, 2016 02:51

add tests

42b6e73

no binder yet

0560bd8

Sync typeshed

7683ee2

Docs: Replace "if False" trick with "if TYPE_CHECKING". (python#2181)

8f0316f

Add column number support (python#2163)

fde83d5

When mypy is run with `--show-column-numbers`, error messages include columns. Column numbers are 0-based. Fixes python#1216.

Fix python#2070 - bytes formatting incorrect in python 3 (python#2168)

2fbf387

Support rebinding on multiassignment from union

da3a516

It's just getting uglier :| Using silenced period for the binder.

Merge remote-tracking branch 'upstream/master' into multiassign_union

ab60317

more tests

6bb5519

Bare Tuple should mean Tuple[Any, ...]. (python#2185)

0d2c594

* Fix python#2184.

Add --recursive and --ignore-errors flags to stubgen (python#2183)

2c73db5

Fix lint

d2c6e8b

elazarg and others added 11 commits October 1, 2016 19:25

Tighten types for parse.py and some more (python#2208)

af0b24a

Make Expression and Statement separable (python#2209)

d5c5c02

Make Expression, Statement true subclasses of Node. See python#1783.

640921b

Add mypy.ini. Fix strict none errors in the test subpackage. (python#…

20aea69

…2200) * Fix strict none errors in the test subpackage. This shows how to deal with python#2199 for `p[i].arg`. * Run mypy over itself in --strict-optional mode. Add mypy.ini to suppress most errors (for now).

Sync typeshed

a3b6546

Document support for Python 3.6 features.

91a2275

Document --show-column-numbers and --scripts-are-modules

bc4a390

Shorten "inherently stale" log message if all nodes in scc are stale

2e6cb95

Fix lint error

d6a7069

RIP weak mode. (python#2217)

dd71ddc

Fixes python#2212. All that's left is some calls to in_checked_function(), which replaces the old typing_mode_full() and typing_mode_none() (its negation after the elimination of typing_mode_weak()).

gvanrossum changed the title ~~Fix #1855: Multiassign from Union~~ [conflict] Fix #1855: Multiassign from Union Oct 4, 2016

elazarg added 12 commits October 5, 2016 17:48

handle union, add test

38651c4

kill blank

9000099

more tests

f20f3d6

handle binding

61be4e9

try to minimize visual difference

9830cb4

(cont.)

59dc8b7

add tests

7f304e4

no binder yet

ff1ca80

Support rebinding on multiassignment from union

168087e

It's just getting uglier :| Using silenced period for the binder.

more tests

3f198cf

Rebase

4fa059f

Merge

ab35a4c

Conflicts: mypy/binder.py mypy/checker.py

elazarg mentioned this pull request Oct 5, 2016

Fix #1855: Multiassign from Union (take 2) #2219

Closed

elazarg closed this Oct 5, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[conflict] Fix #1855: Multiassign from Union #2154

[conflict] Fix #1855: Multiassign from Union #2154

elazarg commented Sep 18, 2016 •

edited

Loading

elazarg Sep 19, 2016

JukkaL Sep 21, 2016

gvanrossum commented Sep 19, 2016

elazarg commented Sep 19, 2016

gvanrossum commented Sep 19, 2016

elazarg commented Sep 19, 2016

JukkaL commented Sep 19, 2016

elazarg commented Sep 19, 2016

JukkaL Sep 21, 2016

JukkaL Sep 21, 2016

elazarg Sep 21, 2016

JukkaL Sep 21, 2016

JukkaL Sep 21, 2016

JukkaL Sep 21, 2016

JukkaL Sep 21, 2016 •

edited

Loading

JukkaL Sep 21, 2016

JukkaL Sep 21, 2016

elazarg Sep 21, 2016

elazarg Sep 27, 2016

elazarg commented Oct 5, 2016

elazarg commented Oct 5, 2016

gvanrossum commented Oct 5, 2016 via email

elazarg commented Oct 5, 2016

[conflict] Fix #1855: Multiassign from Union #2154

[conflict] Fix #1855: Multiassign from Union #2154

Conversation

elazarg commented Sep 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum commented Sep 19, 2016

elazarg commented Sep 19, 2016

gvanrossum commented Sep 19, 2016

elazarg commented Sep 19, 2016

JukkaL commented Sep 19, 2016

elazarg commented Sep 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JukkaL Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elazarg commented Oct 5, 2016

elazarg commented Oct 5, 2016

gvanrossum commented Oct 5, 2016 via email

elazarg commented Oct 5, 2016

elazarg commented Sep 18, 2016 •

edited

Loading

JukkaL Sep 21, 2016 •

edited

Loading