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.

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

Allow subscripted aliases at function scope #4000

Merged
merged 7 commits into from Oct 10, 2017

Conversation

Projects
None yet
2 participants
@ilevkivskyi
Collaborator

ilevkivskyi commented Sep 24, 2017

Fixes #3145

This PR allows subscripted type aliases at function scope, as discussed in #3145 this will simplify the rules and have other pluses (apart from fixing the actual issue).

In addition, I prohibit reuse of bound type variables to define generic aliases, situations like this were always ambiguous:

T = TypeVar('T')
class C(Generic[T]):
    A = List[T]
    x: A
    reveal_type(x)  # on one hand one might expect this to be List[T],
                    # but on the other hand, non-subscripted generic aliases
                    # get variables replaced with 'Any', so this should be 'List[Any]'.

Also prohibiting this use is consistent with how we prohibit bound type variable reuse for nested classes, one can always just use another type variable, so this works:

T = TypeVar('T')
S = TypeVar('S')
class C(Generic[T]):
    A = List[S]
    x: A[T]

@ilevkivskyi ilevkivskyi referenced this pull request Oct 9, 2017

Closed

Release 0.540 planning #4084

@JukkaL JukkaL self-assigned this Oct 9, 2017

@JukkaL

Thanks for the PR! Looks pretty good, but I was confused in a few places.

"""Check if 'rvalue' represents a valid type allowed for aliasing
(e.g. not a type variable). If yes, return the corresponding type and a list of
qualified type variable names for generic aliases.
If 'allow_unnormalized' is True, allow types like builtins.list[T].

This comment has been minimized.

@JukkaL

JukkaL Oct 9, 2017

Collaborator

Does this PR also affect whether list[x] is accepted in stubs?

@JukkaL

JukkaL Oct 9, 2017

Collaborator

Does this PR also affect whether list[x] is accepted in stubs?

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

No, I just noticed that this parameter is unused in the function body when I added the new one.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

No, I just noticed that this parameter is unused in the function body when I added the new one.

@@ -650,7 +650,7 @@ def analyze_type_type_callee(self, item: Type, context: Context) -> Type:
res = type_object_type(item.type, self.named_type)
if isinstance(res, CallableType):
res = res.copy_modified(from_type_type=True)
return res
return expand_type_by_instance(res, item)

This comment has been minimized.

@JukkaL

JukkaL Oct 9, 2017

Collaborator

What's this change?

@JukkaL

JukkaL Oct 9, 2017

Collaborator

What's this change?

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

This is not directly related to this PR, but this was exposed by a test I updated. For example:

x: Type[C[int]]
x()  # this should be 'C[int]', not 'C[<nothing>]'

y: Type[List]
y()  # this should be 'List[Any]', not 'List[<nothing>]'

By the way, this use case is not mentioned in PEP 484, but there are tests for this, for example testClassValuedAttributesGeneric. There are of course subtleties related to Type[...], but I think this change doesn't add unsafety, but removes some false positives.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

This is not directly related to this PR, but this was exposed by a test I updated. For example:

x: Type[C[int]]
x()  # this should be 'C[int]', not 'C[<nothing>]'

y: Type[List]
y()  # this should be 'List[Any]', not 'List[<nothing>]'

By the way, this use case is not mentioned in PEP 484, but there are tests for this, for example testClassValuedAttributesGeneric. There are of course subtleties related to Type[...], but I think this change doesn't add unsafety, but removes some false positives.

Show outdated Hide outdated mypy/semanal.py
@@ -650,7 +650,7 @@ def analyze_type_type_callee(self, item: Type, context: Context) -> Type:
res = type_object_type(item.type, self.named_type)
if isinstance(res, CallableType):
res = res.copy_modified(from_type_type=True)
return res
return expand_type_by_instance(res, item)

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

This is not directly related to this PR, but this was exposed by a test I updated. For example:

x: Type[C[int]]
x()  # this should be 'C[int]', not 'C[<nothing>]'

y: Type[List]
y()  # this should be 'List[Any]', not 'List[<nothing>]'

By the way, this use case is not mentioned in PEP 484, but there are tests for this, for example testClassValuedAttributesGeneric. There are of course subtleties related to Type[...], but I think this change doesn't add unsafety, but removes some false positives.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

This is not directly related to this PR, but this was exposed by a test I updated. For example:

x: Type[C[int]]
x()  # this should be 'C[int]', not 'C[<nothing>]'

y: Type[List]
y()  # this should be 'List[Any]', not 'List[<nothing>]'

By the way, this use case is not mentioned in PEP 484, but there are tests for this, for example testClassValuedAttributesGeneric. There are of course subtleties related to Type[...], but I think this change doesn't add unsafety, but removes some false positives.

"""Check if 'rvalue' represents a valid type allowed for aliasing
(e.g. not a type variable). If yes, return the corresponding type and a list of
qualified type variable names for generic aliases.
If 'allow_unnormalized' is True, allow types like builtins.list[T].

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

No, I just noticed that this parameter is unused in the function body when I added the new one.

@ilevkivskyi

ilevkivskyi Oct 9, 2017

Collaborator

No, I just noticed that this parameter is unused in the function body when I added the new one.

Show outdated Hide outdated mypy/semanal.py
@JukkaL

JukkaL approved these changes Oct 10, 2017

Thanks for the updates! The code is now much easier to read.

@JukkaL JukkaL merged commit 3e49ef9 into python:master Oct 10, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment