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

Revamped generic class behavior to conform to updated PEP 484 #195

Merged
merged 9 commits into from Apr 4, 2016

Conversation

Projects
None yet
2 participants
@gvanrossum
Member

gvanrossum commented Mar 25, 2016

Fixes #115

@ilevkivskyi

This comment has been minimized.

Collaborator

ilevkivskyi commented Mar 25, 2016

It looks great! Still I have found something that I think could be confusing.

First, I would expect that Iterable[T][int] and Iterable[int] is the same type, but Iterable[T][int] == Iterable[int] gives me False.

Second (maybe related) issue is that now __repr__ shows the whole "history" of a type. For example, Iterable[Tuple[T,T]][int] is printed as __main__.Iterable<+T_co>[__main__.Tuple[~T, ~T]]<~T>[int]. This reads as first there was an Iterable parameterized by T_co, then it was indexed by Tuple[T, T] and we had something parameterized by T, and then it was indexed by int. The question is whether we need to expose all this information (that is rather internal) to the user? I think it would be more clear if __repr__ in this case will show something like Iterable[Tuple[int,int]].

@gvanrossum

This comment has been minimized.

Member

gvanrossum commented Mar 25, 2016

Ah, yes. I thought of making Iterable[T][int] == Iterable[int], but I ran out of steam on how to implement it. Then again, thinking about it I'm not sure that it's important to do this -- the runtime behavior of these things is of marginal interest and mypy doesn't let you write that anyway (the PEP is silent on whether this is allowed).

Ditto for repr(). The one important thing about repr() is that when you have

class C(Generic[T]): ...

then repr(C) shows C<~T> rather than C[~T] as it used to. I think the new form is better. And then C[Tuple[int, int]] will actually be printed as C<~T>[Tuple[int, int]]. The longer form (with alternating <...> and [...]) is only of theoretical importance.

Anyway, thanks for the encouragement. I hope others look over this important change too! I intend to land this in Python 3.5.2 (whenever it is released).

@ilevkivskyi

This comment has been minimized.

Collaborator

ilevkivskyi commented Apr 2, 2016

I am now thinking about one additional thing that might be not completely clear: covariance. If I initially have Iterable[T_co] covariant in its type parameter and write

class C(Iterable[Tuple[T,S]]):
    ...

then one could be not sure whether C is covariant in its variables or invariant. I think the natural answer is that covariance is determined only by the final list of type parameters, so that the class above is invariant in both variables. However, I didn't find anywhere in the PEP any explicit explanation of this point. Maybe it is obvious and it is only my problem of understanding, but if not then maybe one should add a short explanation to the PEP?

@gvanrossum

This comment has been minimized.

Member

gvanrossum commented Apr 3, 2016

@ilevkivskyi

This comment has been minimized.

Collaborator

ilevkivskyi commented Apr 3, 2016

I just noticed that this PR also fixes #129 . In general, I think this new "revamped" behavior is much better than the old one.

@gvanrossum gvanrossum merged commit 8c6aaf3 into master Apr 4, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@gvanrossum gvanrossum deleted the new_generic branch Apr 26, 2016

ilevkivskyi added a commit to ilevkivskyi/typehinting that referenced this pull request Aug 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment