Self Type #2193

elazarg · 2016-09-29T03:34:29Z

Implement #1212. I don't know if that's The Right Way To Do It.

There is no attempt to check or disallow abuse of this feature.

elazarg · 2016-09-29T18:24:33Z

@JukkaL It would be nice if you'll skim to see that nothing inherently wrong is going on here.

JukkaL · 2016-09-29T21:34:44Z

I did a very quick pass and the feedback I have is that this needs more time to study than I have right now :-)

I'll take a more careful look tomorrow.

Conflicts: mypy/types.py

elazarg · 2016-09-30T01:13:59Z

mypy/nodes.py

 INVARIANT = 0  # type: int
 COVARIANT = 1  # type: int
-CONTRAVARIANT = 2  # type: int
+# TODO: SELF_VARIANCE = 2  # type: int


What do you think? I am not sure what this is, For now simple INVARIANT seems to suffice

INVARIANT should be fine. I don't see a need for special variance.

elazarg · 2016-09-30T09:01:38Z

mypy/types.py

+        )
+
+
+class TypeVarInstantiator(TypeTranslator):


Is it implemented somewhere else already?

Look at mypy/expandtype.py.

When I use it I get partial types. I must be missing something about how the whole thing works.

The partial types appear only when I use reveal_type, and it seems to be orthogonal to self-type specifically (it happens with normal generic functions too).

My mistake - it was the erased flag. I turned it off for this "inference".

gvanrossum · 2016-10-03T18:46:45Z

I'm trying to kick the tires here and I don't understand why this example doesn't work:

from typing import TypeVar

T = TypeVar('T', bound='C')

class C:
    def copy(self: T) -> T:
        return self

def clone(arg: T) -> T:
    return arg.copy()

The copy() method type-checks wonderfully, but on the very last line I get:

__tmp__.py: note: In function "clone":
__tmp__.py:10: error: Incompatible return value type (got "C", expected "T")

JukkaL · 2016-10-03T18:49:40Z

Sorry, I've been busy with other stuff and haven't been able to review this. Tomorrow should be better!

gvanrossum · 2016-10-03T18:58:08Z

Similar:

from typing import Type, TypeVar

T = TypeVar('T', bound='C')

class C:
    @classmethod
    def new(cls: Type[T]) -> T:
        return cls()

def make(cls: Type[T]) -> T:
    return cls.new()

Gives:

__tmp__.py: note: In function "make":
__tmp__.py:11: error: Incompatible return value type (got "C", expected "T")

JukkaL · 2016-10-04T15:09:22Z

mypy/nodes.py

@@ -1648,9 +1647,10 @@ def accept(self, visitor: NodeVisitor[T]) -> T:
 #
 # If T is contravariant in Foo[T], Foo[object] is a subtype of
 # Foo[int], but not vice versa.
+CONTRAVARIANT = -1  # type: int


The change of the constant value seems unrelated to the rest of diff and unnecessary.

My poor git skills seems to have failed me. I did revert this one.

JukkaL · 2016-10-04T15:14:46Z

mypy/types.py

@@ -698,6 +705,31 @@ def deserialize(cls, data: JsonDict) -> 'CallableType':
                            is_classmethod_class=data['is_classmethod_class'],
                            )

+    def bind_self(self, actual_self: Type = None) -> 'CallableType':


Our convention is to keep Type (and Node) subclasses as almost pure data. This is complex enough that it would be better moved to a module-level function. I suspect you can use a type variable ranging over FunctionLike and CallableType to give it a precise enough type.

JukkaL · 2016-10-04T15:18:14Z

test-data/unit/check-classes.test

+class A:
+    def copy(self: T) -> T:
+        if B():
+            return A()


Try to use the # E: error message convention for the error messages for readability and consistency. If it doesn't work because of all the notes (it shouldn't be a problem here), use a # Error comment on lines that generate an error to make it easier to match errors to lines. You can leave the main: note: In ... in the [out] section and add # E: ... comments here.

JukkaL · 2016-10-26T17:41:55Z

mypy/checkexpr.py

@@ -1608,13 +1608,17 @@ def analyze_super(self, e: SuperExpr, is_lvalue: bool) -> Type:
                        return AnyType()
                    if not self.chk.in_checked_function():
                        return AnyType()
-                    selftype = self.chk.function_stack[-1].arguments[0].variable.type
-                    return analyze_member_access(name=e.name, typ=fill_typevars(e.info), node=e,
+                    # fill_typevars(e.info) erases type variables


This comment isn't right. fill_typevars adds type variables, erasure means something like stripping away type variables or replacing them with the upper bound / Any, which isn't happening here. If I have a class define as class C(Generic[T]): then fill_typevars will create a type C[T] when called with the type info for C as an argument.

So I did get it wrong there. Yet if I pass fill_typevars() as actual_type I get the erasure on access.

I'm completely confused now :\

Oh. wait. it's only confusing because we talk about different variables... sorry. I'll update.

JukkaL · 2016-10-26T18:23:10Z

mypy/checkexpr.py

+                    args = self.chk.function_stack[-1].arguments
+                    # An empty args with super() is an error, but we need something in declared_self
+                    declared_self = args[0].variable.type if args else erased_self
+                    return analyze_member_access(name=e.name, typ=erased_self, node=e,


All the checker tests pass if I pass declared_self as the typ argument. Can we just get rid of erased_self here, as I don't see any use for it? We can return Any without calling analyze_member_access in case there are no arguments (worth generating an error message, though). This would help resolve the actual_self documentation issue in analyze_member_access as well, I believe, as we can rename the argument to original_type and it doesn't need to be special cased for this particular call site. What do you think?

I am not sure this will not break for generic classes. But perhaps we can wait and see.

Yeah, it could well break for generic classes, but as we don't have tests for them we don't know whether they work right now. Let's do that separately. Even for generic classes this is perhaps not the best approach.

JukkaL · 2016-10-27T16:19:41Z

mypy/checkmember.py


-    This is a general operation that supports various different variations:
-
-      1. lvalue or non-lvalue access (i.e. setter or getter access)


Please retain This is general ... and the following numbered list as they are still relevant.

Oops. Sorry.

JukkaL · 2016-10-27T16:31:51Z

Okay, this is almost ready to merge! Thanks for your patience -- this is one of trickiest type system features that I've reviewed recently. It's also going to be pretty useful, I feel.

If you address the comment about the docstring and the test case(https://github.com/python/mypy/pull/2193/files#r85375637) I can merge this. Then one of us can create tasks for follow-up work.

elazarg · 2016-10-27T16:42:17Z

Thank you all - it's been a pleasure :)

JukkaL · 2016-10-27T17:00:50Z

@ilevkivskyi Sorry, I think that I didn't answer your question. I'm still undecided on this issue.

Let's continue the discussion in separate issues that I just created (also feel free to create additional ones for things that I missed): #2351, #2352, #2353 and #2354.

Fixes #2372.

(Reopening #2408 after the revert #2414. This fixes parts of #2090) This PR does three things: Fix the handling of TupleType (pass original_type recursively) Fix the handling of TypeType (avoid ad-hoc buggy special casing) Make NamedTuple's _replace() and _make() return selftype, serving as test case for (1) and (2) As a consequence of (1), some error messages are changed, as exemplified in check-isinstance.test. I think it's better now, but if it isn't, perhaps we should separate original_type and report_type as discussed in #2193.

elazarg added 10 commits September 29, 2016 00:33

move things around

04934c3

first test pass. no override yet

4eb5f75

selftypeClass test pass

181ff09

Simple override

25e104a

Simple override

5e72623

test super() and rename self_type()

29d2e0f

more tests

4d5d479

unneeded changes

ea6c485

merge

00596a6

some more tests

39615c2

recursive instantiation

f8875f9

gvanrossum changed the title ~~Self Type (unfinished)~~ [WIP] Self Type Sep 29, 2016

elazarg added 4 commits September 30, 2016 03:15

add implicit bound

9ae3bf9

add tests: prohibit overriding without selftype

b29bf87

Merge

f097967

Conflicts: mypy/types.py

Merge

747c5b2

Conflicts: mypy/types.py

elazarg commented Sep 30, 2016

View reviewed changes

elazarg added 2 commits September 30, 2016 04:16

minor

9b68a2f

Merge

65d6428

elazarg commented Sep 30, 2016

View reviewed changes

elazarg added 2 commits September 30, 2016 12:10

fix comment

6c161b9

rename: self->cls

1cae034

JukkaL reviewed Oct 4, 2016

View reviewed changes

elazarg added 2 commits October 26, 2016 20:26

avoid crush in super(); document

c152c20

lint

4f2017e

JukkaL reviewed Oct 26, 2016

View reviewed changes

elazarg added 2 commits October 26, 2016 21:18

comment for fill_typevars

fddbba0

comment for fill_typevars

3cf8ecf

JukkaL reviewed Oct 26, 2016

View reviewed changes

elazarg added 3 commits October 26, 2016 21:44

rename actual_self, use declared_type in analyze_super

707da4e

test for erased types

48e139d

lint

25b84f8

elazarg mentioned this pull request Oct 26, 2016

Support iteration on enums #2305

Closed

JukkaL reviewed Oct 27, 2016

View reviewed changes

Revert accidental deletion from doc

8944ff1

add warning for unsafe testcase

9221001

JukkaL merged commit ba85545 into python:master Oct 27, 2016

This was referenced Oct 27, 2016

Check type variable bounds of self types #2351

Open

Decide what to do about variance and self types #2352

Open

Overriding of methods with self types #2353

Open

Self types in generic classes #2354

Open

ddfisher mentioned this pull request Oct 28, 2016

Crash on super call in function with type variable #2361

Closed

elazarg deleted the self_type branch October 28, 2016 00:11

This was referenced Oct 28, 2016

Crash when checking for method override #2372

Closed

Mypy doesn't ensure self types are reasonable #2374

Closed

gvanrossum pushed a commit that referenced this pull request Oct 29, 2016

Fix crash for deferred methods introduced by #2193 (self-type).

7606e5e

Fixes #2372.

ddfisher pushed a commit that referenced this pull request Oct 29, 2016

Fix crash for deferred methods introduced by #2193 (self-type). (#2375)

8a5f463

Fixes #2372.

elazarg mentioned this pull request Nov 11, 2016

Selftype with TupleType #2436

Merged

jboning mentioned this pull request Dec 8, 2016

Generic Self Type #2537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Self Type #2193

Self Type #2193

elazarg commented Sep 29, 2016 •

edited

Loading

elazarg commented Sep 29, 2016

JukkaL commented Sep 29, 2016

elazarg Sep 30, 2016 •

edited

Loading

JukkaL Oct 4, 2016

elazarg Sep 30, 2016

JukkaL Oct 4, 2016

elazarg Oct 4, 2016

elazarg Oct 10, 2016

elazarg Oct 14, 2016

gvanrossum commented Oct 3, 2016

JukkaL commented Oct 3, 2016

gvanrossum commented Oct 3, 2016

JukkaL Oct 4, 2016

elazarg Oct 5, 2016

JukkaL Oct 4, 2016

JukkaL Oct 4, 2016

JukkaL Oct 26, 2016

elazarg Oct 26, 2016

elazarg Oct 26, 2016

JukkaL Oct 26, 2016

elazarg Oct 26, 2016

JukkaL Oct 27, 2016

JukkaL Oct 27, 2016

elazarg Oct 27, 2016

JukkaL commented Oct 27, 2016

elazarg commented Oct 27, 2016

JukkaL commented Oct 27, 2016


		This is a general operation that supports various different variations:

		1. lvalue or non-lvalue access (i.e. setter or getter access)

Self Type #2193

Self Type #2193

Conversation

elazarg commented Sep 29, 2016 • edited Loading

elazarg commented Sep 29, 2016

JukkaL commented Sep 29, 2016

elazarg Sep 30, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum commented Oct 3, 2016

JukkaL commented Oct 3, 2016

gvanrossum commented Oct 3, 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

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 commented Oct 27, 2016

elazarg commented Oct 27, 2016

JukkaL commented Oct 27, 2016

elazarg commented Sep 29, 2016 •

edited

Loading

elazarg Sep 30, 2016 •

edited

Loading