Skip to content

Follow PEP 8 recommendations for type variable names #1872

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

Closed
ilevkivskyi opened this issue Feb 11, 2018 · 3 comments · Fixed by #4898
Closed

Follow PEP 8 recommendations for type variable names #1872

ilevkivskyi opened this issue Feb 11, 2018 · 3 comments · Fixed by #4898
Assignees
Labels
stubs: improvement Improve/refactor existing annotations, other stubs issues

Comments

@ilevkivskyi
Copy link
Member

This appeared in #1856

The problem is that type variable names in some stubs don't follow PEP 8 that recommends short names. This recommendation exists for a reason: behaviour of some constructs is subtly different with type variables and normal types, so it should be immediately clear that a given name refers to a type variable. Most notable examples are generic base classes and generic type aliases, where this leads to false negatives (unless --disallow-any-generics is used):

LongNameLikeAClass = TypeVar('LongNameLikeAClass')
# 300 lines later
class Cls(Iterable[LongNameLikeAClass]):  # it's not obvious this class is generic
    ...
Alias = Dict[str, List[LongNameLikeAClass]]  # even more here, this looks like a normal alias

def fun(arg: Cls) -> Alias:  # two `Any`s sneaked in here
    ...

(it is not something hypothetic, I have seen this kind of errors many times)

I propose to replace the existing long type variable names to follow PEP 8 style. If someone really doesn't like short names, we could propose some other agreement, like end long type variable names with _TVar.

@gvanrossum
Copy link
Member

Agreed, go ahead.

@JelleZijlstra
Copy link
Member

This seems to come up most often with self-types, where we need a per-class typevar like TypeVar(_OrderedDictT, bound=OrderedDict). I try to write those as type name plus T, which should be sufficient to indicate that the name is a typevar.

So as an alternative, I propose that typevar names should always end in T (or T1 or similar). This way, we don't end up using cryptic abbreviations like "_ODT".

@ilevkivskyi ilevkivskyi self-assigned this Feb 13, 2018
@srittau srittau added size-small stubs: improvement Improve/refactor existing annotations, other stubs issues labels Oct 28, 2018
@srittau
Copy link
Collaborator

srittau commented Oct 28, 2018

I just quickly grepped the typeshed sources and this only affects ~20 definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: improvement Improve/refactor existing annotations, other stubs issues
Projects
None yet
4 participants