Skip to content

Alternative (simpler) repr for generics #296

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

Merged
merged 8 commits into from
Oct 20, 2016

Conversation

ilevkivskyi
Copy link
Member

@gvanrossum
Currently, repr for generics acts in naive way, so that sometimes this leads to cumbersome representations. For example,

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

C[Tuple[S, T]][T, int][str]

prints the "whole history" C<~T>[typing.Tuple[~S, ~T]]<~S, ~T>[~T, int]<~T>[str]. While the new repr will print simply the "final result" C[typing.Tuple[str, int]]. Also for simpler cases, repr(List[T]) would be simply typing.List[~T], not typing.List<~T>[~T]<~T>.

The new repr does not use angular brackets in addition to square brackets (free parameters are anyway marked with ~, + or -), see added tests.

The implementation might look a bit hacky, but this is done in order to be sure that the repr will not interfere with those of super() and will treat correctly complex cases with repeated occurrences of type variables.

@gvanrossum
Copy link
Member

I like this. I've been struggling with the repr() quite a bit (in Python 3.5.0 it was quite different). I don't know if I'll have time to merge today but I hope soon.

@gvanrossum
Copy link
Member

I just found a bug here, with very light QA.

from typing import Any, Dict

class C(Dict[Any, Any]):
    pass

print(C.__mro__)

Gives a traceback:

Traceback (most recent call last):
  File "y.py", line 6, in <module>
    print(C.__mro__)
  File "/Users/guido/src/typing/src/typing.py", line 1009, in __repr__
    return super().__repr__() + self._argrepr()
  File "/Users/guido/src/typing/src/typing.py", line 1017, in _argrepr
    for i in range(len(self.__args__)):  # replace free parameters with args
TypeError: object of type 'NoneType' has no len()

@ilevkivskyi
Copy link
Member Author

Oops! I played with this on top of #295 most of the time, so I didn't notice this. Thanks!
I just pushed a new commit where I carefully treat Generic[T] and _Protocol[T] (with more tests).

@ilevkivskyi
Copy link
Member Author

@gvanrossum I have a simple patch for http://bugs.python.org/issue27989 (better representation of function signatures by pydoc). I didn't submit it, because it works much better on top of this PR.

Are you going to merge this or you have some additional comments?

@gvanrossum
Copy link
Member

gvanrossum commented Oct 14, 2016 via email

@ilevkivskyi
Copy link
Member Author

@gvanrossum

Sorry, just temporarily ran out of time. Will get to it soon.

Not a problem at all.
In the meantime I implemented the PEP 526 syntax for NamedTuple in mypy: python/mypy#2245 so that if mypy PR is merged, then you could also apply the patch in http://bugs.python.org/issue28107 (earlier we decided to postpone documenting new NamedTuple syntax until it is implemented in mypy).

return '[%s]' % (', '.join(map(_type_repr, self.__parameters__)))
r = self.__origin__._argrepr()
return par_repr
r = self.__origin__._arg_repr()
for i in range(len(self.__args__)): # replace free parameters with args
par = stdlib_re.escape(_type_repr(self.__origin__.__parameters__[i]))
r = stdlib_re.sub(par + '(?=[,\]])', '{%r}' % i, r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry for only just now looking at this code in detail. I really don't like the idea that we're taking some other class's _arg_repr() and then doing a regex substitution on it. Especially since this is just calling itself recursively. There's got to be a better way.

@ilevkivskyi
Copy link
Member Author

@gvanrossum
Agreed, probably I am just being lazy here :-) Now there is no regexp and no recursion (except for the case one calls repr(List[List[List[T]]][int]), in this case there will be three recursive calls). It was necessary to add a _subs_repr method to some types that can contain type variables but are not instances of GenericMeta (this is similar to what you did with _get_type_vars). Note that no new types are created, all manipulations are done with lists of parameters/arguments.

While working on this, I have found three very minor bugs (I have fixed them right here) and three bigger things that I think are bugs, I will open issues on tracker soon.

@ilevkivskyi
Copy link
Member Author

@gvanrossum I forgot to add, I changed the repr for bare generics, now repr(List) == 'typing.List', not typing.List[~T] as before. There are two reasons for this:

  • It is more consistent with other things in typing. For example repr(Optional) is simply 'typing.Optional', etc.
  • By agreement, bare List means List[Any], so that if one wants a function that takes an arbitrary list and returns an arbitrary list, then it would be typed as def func(x: List) -> List. But then in pydoc and other doc-tools its signature will be displayed as func(x: List[~T]) -> List[~T], that looks like a generic function. I could imagine this causing a lot of confusion.

@gvanrossum
Copy link
Member

gvanrossum commented Oct 16, 2016 via email

@gvanrossum
Copy link
Member

I'm giving you the benefit of the doubt here -- it looks fine but I'm out of time this week to really understand how this works. I trust it's easy enough to understand when someone finds a bug though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants