Skip to content
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

Signature of ItemsView in typing.py does not match that in typeshed's typing.pyi #177

Closed
gvanrossum opened this issue Jan 25, 2016 · 3 comments

Comments

@gvanrossum
Copy link
Member

This came up in #136 (comment).

There seems to be a bug in the definition of ItemsView in typing.py: it has three type parameters, but its unclear what they mean. When looking at the corresponding definition in typeshed, it actually makes more sense:

class MappingView(Sized):
    def __len__(self) -> int: ...

class ItemsView(AbstractSet[Tuple[_KT_co, _VT_co]], MappingView, Generic[_KT_co, _VT_co]):
    def __contains__(self, o: object) -> bool: ...
    def __iter__(self) -> Iterator[Tuple[_KT_co, _VT_co]]: ...

Here MappingView is not generic (since Sized isn't), and ItemsView has only two parameters, clearly meant for keys and values, and clarified by the __contains__ and __iter__ signatures.

I think we should follow this example.

@gvanrossum
Copy link
Member Author

I think this was fixed by my fix for #115. The mapping view objects are now defined as follows:

class MappingView(Sized, Iterable[T_co], extra=collections_abc.MappingView):
    pass


class KeysView(MappingView[KT], AbstractSet[KT],
               extra=collections_abc.KeysView):
    pass


class ItemsView(MappingView[Tuple[KT, VT_co]],
                Set[Tuple[KT, VT_co]],
                Generic[KT, VT_co],
                extra=collections_abc.ItemsView):
    pass


class ValuesView(MappingView[VT_co], extra=collections_abc.ValuesView):
    pass

(The Generic[KT, VT_col] base in ItemsView is technically redundant, but it's there as an example. It should have no effect.)

@ilevkivskyi
Copy link
Member

I have a questions here. What is the reason to have Set in bases for ItemsView, probably it should be AbstractSet. At least in collections.abc both ItemsView and KeysView inherit from collections.abc.Set. Therefore in both cases it is reasonable to use typing.AbstractSet which is a generic version of collections.abc.Set.

@gvanrossum
Copy link
Member Author

Good catch! Probably a sleepy copy/paste from collections.abc. Will fix.

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

No branches or pull requests

2 participants