Generic type aliases #2378

ilevkivskyi · 2016-10-29T12:34:41Z

Fixes #606 as per PEP 484.

Now type aliases could be generic, so that the example in PEP now works. Generic type aliases are allowed for generic classes, unions, callables, and tuples, for example:

Vec = Iterable[Tuple[T, T]]
TInt = Tuple[T, int]
UInt = Union[T, int]
CBack = Callable[..., T]

The aliases could be used as specified in PEP 484, e.g. either one specifies all free type variables, or if unspecified they are all substituted by Any, for example:

def fun(v: Vec[T]) -> Vec[T]: # Same as Iterable[Tuple[T, T]]
    ...
v1: Vec[int] = []      # Same as Iterable[Tuple[int, int]]
v2: Vec = []           # Same as Iterable[Tuple[Any, Any]]
v3: Vec[int, int] = [] # Error: Invalid alias, too many type arguments!

Generic aliases could be used everywhere, where a normal generic type could be used (in annotations, generic base classes etc, and since recently in runtime expressions). Nested and chained aliases are allowed, but excessive use of those is discouraged in the docs. As ordinary (non-generic) aliases, generic ones could be imported from other modules.

I believe this is a useful feature (I found three issues on tracker requesting this). When someone wants precise types, they quickly become quite verbose, so that generic type aliases will improve readability. Please, don't be scared by the size of diff, the actual implementation is quite simple, 85% of changes are extensive tests.

NOTE: Many examples in the tests and in docs require a recent version of typing.py from python/typing to work at runtime (see #2382). This feature could be used with older versions of typing.py by using type comments or "forward references".

JukkaL · 2016-10-31T14:25:12Z

@gvanrossum @ddfisher Can one of you review this?

gvanrossum · 2016-10-31T15:02:04Z

I can, and will, in due time.

…

--Guido (mobile)

gvanrossum

This is a great piece of work! I think I understand everything that's going on (except I haven't studied the new tests much). Here are a lot of comments, mostly nits. Thanks!

gvanrossum · 2016-10-31T17:52:14Z

docs/source/kinds_of_types.rst

-for another type -- it's equivalent to the target type. Type aliases
-can be imported from modules like any names.
+Type aliases can be generic, in this case they could be used in two variants:
+Subscribed aliases are equivalent to original types with substituted type variables,


Subscripted

gvanrossum · 2016-10-31T17:52:36Z

docs/source/kinds_of_types.rst

+Subscribed aliases are equivalent to original types with substituted type variables,
+number of type arguments must match the number of free type variables
+in generic type alias. Unsubscribed aliases are treated as original types with free
+vaiables replacec with ``Any``. Examples (following `PEP 484


gvanrossum · 2016-10-31T17:54:42Z

docs/source/kinds_of_types.rst

+    from typing import TypeVar, Iterable, Tuple, Union, Callable
+    T = TypeVar('T', int, float, complex)
+
+    TInt = Tuple[T, int]


There seem to be no examples using TInt, UInt, CBack?

gvanrossum · 2016-10-31T17:59:46Z

docs/source/kinds_of_types.rst

+    A type alias does not create a new type. It's just a shorthand notation for
+    another type -- it's equivalent to the target type. For generic type aliases
+    this means that variance of type variables used for alias definition does not
+    allpy to aliases. Parameterized generic alias is treated simply as an original


gvanrossum · 2016-10-31T18:03:42Z

docs/source/kinds_of_types.rst

+    allpy to aliases. Parameterized generic alias is treated simply as an original
+    type with corresponding type variables substituted. Accordingly, type checking
+    happens when a type alias is used. Invalid aliases (like e.g.
+    ``Callable[..., List[T, T]]``) might not always be flagged by mypy if they are


That would be a regression IMO -- while this is currently just illegal, List[int, int] in an alias is immediately flagged as an error ("Too many parameters for typing.List; actual 2, expected 1") and I'd be sad if that was no longer flagged on the alias line.

@gvanrossum I don't think so. I have just checked and A = Callable[..., List[int, int]] passes without errors with mypy/master. But anyway, it looks like I fixed it, with this PR both the above and aliases likeUnion[int, List[T, T]] etc. should be flagged as errors even if they are left unused.

gvanrossum · 2016-10-31T22:01:05Z

mypy/typeanal.py

@@ -40,6 +40,9 @@ def analyze_type_alias(node: Expression,
    # that we don't support straight string literals as type aliases
    # (only string literals within index expressions).
    if isinstance(node, RefExpr):
+        if node.kind == UNBOUND_TVAR or node.kind == BOUND_TVAR:
+            fail_func('Invalid type "{}" for aliasing'.format(node.fullname), node)


I couldn't find any tests revealing this message?

@gvanrossum Good catch! Added a test.

Could you make the error message clearer? I think it should explicitly state that you cannot create an alias for a type variable.

@gvanrossum OK, now the error explicitly says that type variable is invalid as target for type alias.

gvanrossum · 2016-10-31T22:02:12Z

mypy/typeanal.py

+                if exp_len == 0 and act_len == 0:
+                    return override
+                if act_len != exp_len:
+                    # TODO: Detect wrong type variable numer for unused aliases


Typo: numer.

gvanrossum · 2016-10-31T22:02:33Z

mypy/typeanal.py

@@ -153,7 +174,9 @@ def visit_unbound_type(self, t: UnboundType) -> Type:
                    # as a base class -- however, this will fail soon at runtime so the problem
                    # is pretty minor.
                    return AnyType()
-                self.fail('Invalid type "{}"'.format(name), t)
+                # Allow unbount type variables when defining an alias


Typo: unbount.

gvanrossum · 2016-10-31T22:04:44Z

mypy/typeanal.py

+                    return override
+                if act_len != exp_len:
+                    # TODO: Detect wrong type variable numer for unused aliases
+                    # (it is difficult at this stage, see comment below, line 187 atm)


I'm not crazy about the line number reference... :-)

gvanrossum · 2016-10-31T22:08:48Z

mypy/checkexpr.py

+        a runtime expression. Basically, this function finishes what could not be done
+        in similar funtion from typeanal.py.
+        """
+        if not isinstance(tp, (Instance, UnionType, TupleType, CallableType)):


I'm wondering if this might be a job for TypeTranslator... Or perhaps refactor the code that extracts the type variables from a type into a helper function? (I see you have the exact same idiom in three places now. If in the future we add a new parameterized Type subclass we'd be in a bit of pain finding all places where this is used.

@gvanrossum I refactored this to two helpers get_typ_args() and set_typ_args() in types.py. This indeed made everything more compact, those two are used in both typeanal.py and checkexpr.py.

…eport same error as without alias

…e tests

ilevkivskyi · 2016-11-02T09:20:58Z

@gvanrossum Thank you for a thorough review! And sorry for all the typos, I promise to use a spellchecker. I implemented requested changes in new commits (more details are in replies to individual comments). Please, take a look!

gvanrossum

Approaching the limit! I started a bunch of reviews in multiple tabs, hopefully this is all of them.

gvanrossum · 2016-11-02T20:24:53Z

mypy/checkexpr.py

@@ -6,7 +6,9 @@
    Type, AnyType, CallableType, Overloaded, NoneTyp, Void, TypeVarDef,
    TupleType, Instance, TypeVarId, TypeVarType, ErasedType, UnionType,
    PartialType, DeletedType, UnboundType, UninhabitedType, TypeType,
-    true_only, false_only, is_named_instance, function_type
+    true_only, false_only, is_named_instance, function_type,
+    get_typ_args, set_typ_args


Add trailing comma, remove following blank line.

gvanrossum · 2016-11-02T20:26:13Z

mypy/nodes.py

@@ -1732,13 +1732,13 @@ class TypeAliasExpr(Expression):
    """Type alias expression (rvalue)."""

    type = None  # type: mypy.types.Type
-    line = None  # type: int
+    fback = None  # type: mypy.types.Type


Would be nice to have a comment that clarifies what "fback" means. (Apparently fallback? Why not spell it in full?)

@gvanrossum OK, changed to fallback and added comment.

gvanrossum · 2016-11-02T20:27:53Z

mypy/semanal.py

+        kind = (' to Callable' if isinstance(tp, CallableType) else
+                ' to Tuple' if isinstance(tp, TupleType) else
+                ' to Union' if isinstance(tp, UnionType) else '')
+        cdef = ClassDef('Type alias{}'.format(kind), Block([]))


You can just use 'a' + b instead of 'a{}'.format(b). :-)

gvanrossum · 2016-11-02T20:28:48Z

mypy/semanal.py

+        """Make a dummy Instance with no methods. It is used as a fallback type
+        to detect errors for non-Instance aliases (i.e. Unions, Tuples, Callables).
+        """
+


You don't need a blank line here (the stand-alone """ serves as enough of a visual separator).

gvanrossum · 2016-11-02T20:29:57Z

mypy/typeanal.py

@@ -5,7 +5,7 @@
 from mypy.types import (
    Type, UnboundType, TypeVarType, TupleType, UnionType, Instance,
    AnyType, CallableType, Void, NoneTyp, DeletedType, TypeList, TypeVarDef, TypeVisitor,
-    StarType, PartialType, EllipsisType, UninhabitedType, TypeType
+    StarType, PartialType, EllipsisType, UninhabitedType, TypeType, get_typ_args, set_typ_args


Trailing comma.

gvanrossum · 2016-11-02T20:34:51Z

mypy/typeanal.py

@@ -40,6 +40,9 @@ def analyze_type_alias(node: Expression,
    # that we don't support straight string literals as type aliases
    # (only string literals within index expressions).
    if isinstance(node, RefExpr):
+        if node.kind == UNBOUND_TVAR or node.kind == BOUND_TVAR:
+            fail_func('Invalid type "{}" for aliasing'.format(node.fullname), node)


Could you make the error message clearer? I think it should explicitly state that you cannot create an alias for a type variable.

gvanrossum · 2016-11-02T20:36:57Z

docs/source/kinds_of_types.rst

-    ``Callable[..., List[T, T]]``) might not always be flagged by mypy if they are
-    left unused.
+    this means that variance or constraints of type variables used for alias
+    definition don't apply to aliases. Parameterized generic alias is treated


A parameterized generic alias ...

gvanrossum · 2016-11-02T20:37:23Z

docs/source/kinds_of_types.rst

-    left unused.
+    this means that variance or constraints of type variables used for alias
+    definition don't apply to aliases. Parameterized generic alias is treated
+    simply as an original type with corresponding type variables substituted.


...with the corresponding ...

gvanrossum · 2016-11-02T20:42:31Z

test-data/unit/check-generics.test

@@ -766,7 +766,7 @@ if not isinstance(s, str):

 z = None # type: TNode # Same as TNode[Any]
 z.x
-z.foo() # E: Some element of union has no attribute "foo"
+z.foo() # Any simplyfies Union to Any


Actually, Union[Any, int] is not the same as Any. This is subtle (I got it wrong in an early version of typing.py) but it's important, since when it's an int, it can't be used as a string. See PR #2197

@gvanrossum Yes, I remember our conversation on python/typing tracker. I just wanted to mark somehow this test, so that when it will start failing after Union[Any, ...] PR is merged it will be clear that nothing is special here. I have expanded this comment now to clarify.

gvanrossum · 2016-11-02T21:00:46Z

test-data/unit/check-generics.test


 [builtins fixtures/list.pyi]
+[out]
+main:8: error: Invalid type argument value for "A"
+main:8: error: Type argument 1 of "A" has incompatible value "str"


I don't like getting this twice. The first error is too vague since it doesn't tell you which position has the error.

(Unless you turn on column numbers? Which reminds me, it would be nice to turn on --show-column-numbers for at least some of the tests, to verify that they're being passed along.)

@gvanrossum OK, I updated the error messages, only the more detailed one is shown wherever possible.

Concerning the column numbers I discovered that a lot of code does not copy them. For example, exprtotype.py was simply throwing them away (I fixed this) while this file is the key, if column for a type is ignored here it is lost forever. I investigated this more and I found that this is a limitation of old parser and type comments. I don't know how to fix this (and think it will be difficult), but for fast parser and type annotations it is just one line and we get column for free.

I used --show-column-numbers in two tests (one with old parser and one with fast parser). And indeed fast parser works better. So this is another argument to make the fast parser default.

ilevkivskyi · 2016-11-03T09:08:26Z

@gvanrossum Thank you for review! I implemented the comments in new commits. I hope everything is fine now.

ilevkivskyi · 2016-11-03T09:18:55Z

test-data/unit/check-newtype.test

 main:3: error: Argument 2 to NewType(...) must be subclassable (got T?)
+main:3: error: Invalid type "__main__.T"


@gvanrossum The order of these two messages is changed because now the column is correctly passed and errors are ordered by column.

ilevkivskyi · 2016-11-03T09:20:38Z

test-data/unit/check-generics.test

+[out]
+main:9:7: error: Type argument 1 of "A" has incompatible value "str"
+main:13: error: Type argument 1 of "A" has incompatible value "str"
+main:13: error: Type argument 2 of "A" has incompatible value "str"


@gvanrossum The column is not shown here because the type is inside a comment. For type annotation everything works.

gvanrossum · 2016-11-03T19:50:37Z

Woot! Thanks so much!

gvanrossum · 2016-11-08T02:00:28Z

Sadly this broke something. I can work around it but it would be good if we fixed it before the next public release.

ilevkivskyi mentioned this pull request Oct 29, 2016

Refactor Union, Tuple, and Callable python/typing#308

Merged

gvanrossum requested changes Oct 31, 2016

View reviewed changes

Ivan Levkivskyi and others added 16 commits November 2, 2016 00:35

Initial crude implementation of GA

5dc0090

Add support for Union, Tuple, Callable

f6d862b

Add runtime behaviour; outline tests

b0790d4

Formatting + better types

7ddd6ab

Add some tests

633e3d1

More tests, better error reporting

07baa51

Add tests, comments, and doctrings

56b7ae7

Add even more tests, add documentation

cbcb2c0

Last tests

5c92b12

Corrections due to upstream refactoring

13e1fff

Add few more examples to docs

ee1c25c

Do not substitute type variables if they are in runtime expression, r…

2edcf48

…eport same error as without alias

First part of response to comments (bigger things)

64fc96e

Update tests; some formatting

8c64111

Second part of response to comments: minor things (but many); Two mor…

c106204

…e tests

Fix last bits

feb9413

ilevkivskyi force-pushed the new-generic-aliases branch from e58eff7 to feb9413 Compare November 1, 2016 23:40

ilevkivskyi added 2 commits November 2, 2016 00:53

Minor correction to docs

d402a68

Yet another tiny correction to docs

8cea156

gvanrossum reviewed Nov 2, 2016

View reviewed changes

ilevkivskyi and others added 2 commits November 3, 2016 01:44

Improving error messages (+column numbers); main part

382c53e

Remaining part of response to comments (minor thigs)

863c576

Remove debugging print()

544ff66

ilevkivskyi commented Nov 3, 2016

View reviewed changes

gvanrossum approved these changes Nov 3, 2016

View reviewed changes

gvanrossum merged commit 57ff82b into python:master Nov 3, 2016

gvanrossum mentioned this pull request Nov 8, 2016

Adding # type: ignore to EnumClass[string_value] doesn't set the type to Any #2416

Closed

gvanrossum added this to the 0.4.x milestone Nov 8, 2016

ilevkivskyi mentioned this pull request Nov 8, 2016

Aliases for generic class applications get type Any #2349

Closed

gvanrossum mentioned this pull request Jan 13, 2017

Flaw with type aliases containing type variables #2690

Closed

		main:3: error: Argument 2 to NewType(...) must be subclassable (got T?)
		main:3: error: Invalid type "__main__.T"

Generic type aliases #2378

Generic type aliases #2378

Conversation

ilevkivskyi commented Oct 29, 2016 • edited Loading

JukkaL commented Oct 31, 2016

gvanrossum commented Oct 31, 2016 via email

gvanrossum left a 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

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

ilevkivskyi commented Nov 2, 2016

gvanrossum left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum commented Nov 3, 2016

gvanrossum commented Nov 8, 2016

ilevkivskyi commented Oct 29, 2016 •

edited

Loading