classification
Title: Add Collection to collections.abc and typing
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: bar.harel, brett.cannon, gvanrossum, levkivskyi, neil.g, python-dev, rhettinger, srkunze
Priority: normal Keywords: patch

Created on 2016-07-23 15:46 by brett.cannon, last changed 2016-08-23 19:30 by levkivskyi. This issue is now closed.

Files
File name Uploaded Description Edit
collection.diff levkivskyi, 2016-08-19 00:26 review
doc_changes.diff neil.g, 2016-08-19 06:06 review
collection_neil_combined.diff levkivskyi, 2016-08-19 09:11 review
Messages (26)
msg271088 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-07-23 15:46
See the discussion on python-ideas entitled "An ABC representing "Iterable, Sized, Container"".
msg271094 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-07-23 17:41
I'm thinking that it should be called Collection after all.

--Guido (mobile)
msg271110 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-07-23 20:32
Does all of the recognition patterns in typing.py also need to be in collections.abc? 

AFAICT, the current collections.abc classes for Sized, Iterable, and Container almost never get used (possibly because they don't provide any useful mixin methods).  In the interest of maximizing the ratio of useful ABCs (like MutableMapping), could we defer expanding collections.abc until we've seen some real use cases?
msg271112 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-07-23 21:09
It would be a deviation from existing practice for a collection ABC to
exist in typing but not in collections.abc, and it would mean the
inheritance tree for the types in typing.py would be different from
that in collections.abc.

These ABCs, while not so useful by themselves, are building blocks for
the more useful ABCs. They also give people the right terminology.
That arguably is the biggest problem here -- it's clear that people
desperately want to be able to talk about the common API for sets and
sequences, which happens to be __len__, __iter__ and __contains__. But
it's also clear that people often believe that that common API is
called Iterable, which it isn't.
msg271170 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-07-24 16:44
In putting together a patch for this, I noticed that the __subclasshook__ classmethods don't compose well.   I'm unclear about the semantics about what these hooks are supposed to mean for subclasses and whether there is a bug in the existing code for Set, Sequence, and Mapping.  Each of those inherits from Sized, Iterable, and Container, but only the Sized.__subclasshook__ is visible to the subclass.  Also, the code for Sized.__subclasshook__ has an identity check for Sized which doesn't seem correct from the point-of-view of the subclasses.
msg271191 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-07-25 00:36
Regarding the design of __subclasshook__, these two flaws kind of cancel each other out. The idea is that if you have a "one-trick pony" class whose issubclass() check must verify that the purported sublass implements a specific method (e.g. __hash__), you don't want that subclass check to be inherited by a subclass. I suppose perhaps this ought to have been done by making ABCMeta.__subclasscheck__ check for the __subclasshook__ in the class dict, but (for reasons I've forgotten) it's not doing it that way, instead just calling cls.__subclasscheck__. All known __subclasshook__ implementations (those in collections.abc anyway :-) compensate for this by returning NotImplemented if the class for which they are being called isn't exactly the class in which they are defined. The final piece of the puzzle is object.__subclasshook__(), which always returns NotImplemented.

(NOTE: When reading the above paragraph, be careful to distinguish between __subclasscheck__, which is invoked by issubclass() and isinstance(), and __subclasshook__, which is only invoked by ABCMeta.__subclasscheck__().)

Here's an example showing why we don't want __subclasshook__ to be inherited. Suppose we have a class SupportsInt, like this:

class SupportsInt(ABC):
    @classmethod
    def __subclasshook__(cls, C):
        return hasattr(C, '__int__')

Now suppose we had a concrete class inheriting from this:

class MyInteger(SupportsInt):
    def __int__(self):
        return 0

Now, alas, everything with an __int__ method is considered to be a MyInteger, for example isinstance(0, MyInteger) returns True.

So what should Collection.__subclasshook__ do? It could do something like this:

class Collection(Set, Iterable, Container):
    @classmethod
    def __subclasshook__(cls, C):
        if cls is not Collection:
            return NotImplemented
        for base in Set, Iterable, Container:
            ok = base.__subclasshook__(C)
            if ok != True: return False
        return True

(Untested, hopefully you get the idea. Hope this helps.)
msg271963 - (view) Author: Sven R. Kunze (srkunze) Date: 2016-08-04 09:40
I am way too late to the discussion but I also support the term "collections". I think it also helps developers coming from a C# background:

https://msdn.microsoft.com/library/92t2ye13(v=vs.110).aspx
msg273009 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-18 01:39
Ping -- I'd really like to see this happening, with "Collection" as the name.
msg273012 - (view) Author: Neil Girdhar (neil.g) * Date: 2016-08-18 06:08
@gvanrossum is there any reason that subclasshook is implemented by overriding instead of cooperation?  E.g.,:

class Sized(metaclass=ABCMeta):

    @classmethod
    def __subclasshook__(cls, C):
        return (super().__subclasshook__(C) and 
                any("__len__" in B.__dict__ for B in C.__mro__))


etc.  And then Collection does not need to implement subclasshook since its base classes cooperate?
msg273037 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-18 15:01
Why don't you give it a try? I'd be happy to review a patch from you.
msg273038 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-18 15:16
I thought about this some more. It won't work because when a class method
is called via super(), 'cls' is still set to the derived class (and in
general for class methods that's the right thing to do, just as it is for
regular methods that self is still the actual object). If you read my
previous long comment you'll understand that all the __subclasshook__
methods carefully return NotImplemented immediately if their 'cls' argument
is not the class where they are defined. So it won't work.

I also don't see the use case (you're not supposed to do this at home,
basically).
msg273069 - (view) Author: Ivan Levkivskyi (levkivskyi) * Date: 2016-08-19 00:26
I submit a simple solution in line with __subclasshook__ of other ABCs.
I added several tests, it looks like it is doing everything right.

Please review.
msg273075 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-19 04:30
LGTM, but I'm on vacation through Monday.
msg273079 - (view) Author: Neil Girdhar (neil.g) * Date: 2016-08-19 05:04
Great patch.  Shouldn't Sequence be a "Reversible, Collection"?
msg273082 - (view) Author: Neil Girdhar (neil.g) * Date: 2016-08-19 06:06
(added the documentation changes)
msg273102 - (view) Author: Ivan Levkivskyi (levkivskyi) * Date: 2016-08-19 09:11
Thank you Neil, I agree that Sequence is a reversible collection.

I combined you documentation patch with updated _collections_abc and updated test_functools (it tests MRO) into a single patch.
msg273116 - (view) Author: Neil Girdhar (neil.g) * Date: 2016-08-19 13:16
Given issue http://bugs.python.org/issue27802, it might be worth considering that all Collections implement __eq__ and __ne__, so maybe these should be abstract methods on Collection?
msg273164 - (view) Author: Neil Girdhar (neil.g) * Date: 2016-08-19 23:26
(never mind about the comparison operators :)  Turns out that would break backwards compatibility.)
msg273228 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-20 17:18
No on adding __eq__.

If another core dev is available please go ahead and commit.
msg273467 - (view) Author: Bar Harel (bar.harel) * Date: 2016-08-23 15:45
I still believe "Reiterable" better demonstrates the concept.

When you request a Reiterable as a function parameter or assert if something is a Reiterable the other side knows exactly what you mean.

A "Collection" is way more ambiguous - if you create an object that acts like range() but you can cycle over it more than once I wouldn't exactly call it a collection. I would though call it a Reiterable and it would be clear for any Python programmer familiar with the concept of iterators.

I believe this is a funny case in which the naming is more important than the implementation as it will turn into a term or concept that will be further used in many places to come.
msg273478 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-23 16:28
ReIterable and Collection are different concepts.
- ReIterable just implements __iter__ but promises certain semantics.
- Collection implements __iter__, __contains__ and __len__.

If we had ReIterable I would probably decree that Collection inherits from it (practicality beats purity).

But I don't think ReIterable by itself would do much good; apart from a few "show-off" hacks, any reasonable ReIterable would also be able to implement __contains__ and __len__ easily.

And I would certainly call range() a Collection.
msg273494 - (view) Author: Roundup Robot (python-dev) Date: 2016-08-23 17:47
New changeset acd9a465b373 by Guido van Rossum in branch 'default':
Issue 27598: Add Collections to collections.abc.
https://hg.python.org/cpython/rev/acd9a465b373
msg273495 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-23 17:47
I'll keep this open until I've also merged typing.py.
msg273499 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-23 18:07
changeset:   102867:355fa8bd8d9d
branch:      3.5
parent:      102860:e3466a556d81
user:        Guido van Rossum <guido@python.org>
date:        Tue Aug 23 11:01:50 2016 -0700
summary:     A new version of typing.py from https://github.com/python/typing.

changeset:   102868:013bb690c24a
tag:         tip
parent:      102866:acd9a465b373
parent:      102867:355fa8bd8d9d
user:        Guido van Rossum <guido@dropbox.com>
date:        Tue Aug 23 11:06:30 2016 -0700
summary:     A new version of typing.py from https://github.com/python/typing. (Merge 3.5->3.6)
msg273500 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-23 18:09
Actually there's one TODO left: the docs for typing (at least in 3.6)
should mention Collection.
msg273512 - (view) Author: Ivan Levkivskyi (levkivskyi) * Date: 2016-08-23 19:30
Guido, thank you for committing! There are also some other omissions in docs, Type[C], @overload, there is #26141 to track this. I will submit a patch later. Until 3.6beta1 I would like to focus on PEP 526.
History
Date User Action Args
2016-08-23 19:30:53levkivskyisetmessages: + msg273512
2016-08-23 18:09:22gvanrossumsetmessages: + msg273500
2016-08-23 18:08:18gvanrossumsetstage: test needed -> resolved
2016-08-23 18:07:48gvanrossumsetstatus: open -> closed
type: enhancement
resolution: fixed
messages: + msg273499
2016-08-23 17:47:53gvanrossumsetmessages: + msg273495
2016-08-23 17:47:25python-devsetnosy: + python-dev
messages: + msg273494
2016-08-23 16:29:14gvanrossumsettitle: Add SizedIterable to collections.abc and typing -> Add Collection to collections.abc and typing
2016-08-23 16:28:33gvanrossumsetmessages: + msg273478
2016-08-23 15:45:58bar.harelsetnosy: + bar.harel
messages: + msg273467
2016-08-20 17:18:33gvanrossumsetmessages: + msg273228
title: Add Collection to collections.abc and typing -> Add SizedIterable to collections.abc and typing
2016-08-19 23:26:36neil.gsetmessages: + msg273164
2016-08-19 13:29:49neil.gsettitle: Add SizedIterable to collections.abc and typing -> Add Collection to collections.abc and typing
2016-08-19 13:16:14neil.gsetmessages: + msg273116
2016-08-19 09:11:21levkivskyisetfiles: + collection_neil_combined.diff

messages: + msg273102
2016-08-19 06:06:23neil.gsetfiles: + doc_changes.diff

messages: + msg273082
2016-08-19 05:04:16neil.gsetmessages: + msg273079
2016-08-19 04:30:03gvanrossumsetmessages: + msg273075
2016-08-19 00:26:50levkivskyisetfiles: + collection.diff
keywords: + patch
messages: + msg273069
2016-08-18 15:16:55gvanrossumsetmessages: + msg273038
2016-08-18 15:01:17gvanrossumsetmessages: + msg273037
2016-08-18 06:08:47neil.gsetnosy: + neil.g
messages: + msg273012
2016-08-18 01:39:43gvanrossumsetmessages: + msg273009
2016-08-04 09:40:50srkunzesetnosy: + srkunze
messages: + msg271963
2016-07-28 21:55:29levkivskyisetnosy: + levkivskyi
2016-07-25 00:36:23gvanrossumsetmessages: + msg271191
2016-07-24 16:44:39rhettingersetmessages: + msg271170
2016-07-23 21:09:17gvanrossumsetmessages: + msg271112
2016-07-23 20:32:15rhettingersetnosy: + rhettinger
messages: + msg271110
2016-07-23 17:41:08gvanrossumsetmessages: + msg271094
2016-07-23 15:46:49brett.cannoncreate