Initially, BaseSelector was simply designed as the base implementation used by concrete ones like SelectSelector & Co.

Then BaseSelector evolved to be an ABC, but the problem is that it's really not usable as such: the register() and unregister() methods are not abstract, and instead store the fileobj -> key association in a private dictionary (_fd_to_key). Since this attribute is private, it cannot be used by third-party selectors implementation which might want to implement the ABC. Also, such implementations might not want to use a dictionay internally, and generally, inheritance should be avoided in this type of situations (since it breaks encapsulation).

In short, BaseSelector mixes up the type definition (ABC) and base implementation, which cannot be reused by subclasses anyway.

The attached patch cleans things up by making BaseSelector.{register,unregister,get_map} methods abstract (raising NotImplementedError by default).
Together with select(), those methods are the bare minimum that a conform selector implementation should provide.
get_key() still has a default implementation (atop get_map()), and so does modify() (atop register()/unregister()).

The concrete base implementation (on top of which are built SelectSelector & friends) is moved in a private _BaseSelectorImpl.

I think that's a cleaner design.

The only problem is that it makes some methods abstract, so I had to update test_telnetlib and asyncio/test_utils because they are implementing BaseSelector for mock tests.

BTW, is there a consensus on ABC names? Like AbstractSelector vs BaseSelector?

I'm wondering, is there a reason we made BaseSelector a public API?

> I'm wondering, is there a reason we made BaseSelector a public API?

The idead was to have an ABC so that users can implement their own
selector, and pass it to e.g. asyncio or anything alse expecting a
selector.
Other than that, the only use is as a documentation (i.e. to show
which methods are supported by all selectors implementations).

> The idead was to have an ABC so that users can implement their own
> selector, and pass it to e.g. asyncio or anything alse expecting a
> selector.
> Other than that, the only use is as a documentation (i.e. to show
> which methods are supported by all selectors implementations).

The problem for documentation use is that we're christening it as an
official API, and thus it becomes more difficult to refactor the
inheritance hierarchy.

> The problem for documentation use is that we're christening it as an
> official API, and thus it becomes more difficult to refactor the
> inheritance hierarchy.

So what would you suggest?

> > The problem for documentation use is that we're christening it as an
> > official API, and thus it becomes more difficult to refactor the
> > inheritance hierarchy.
> 
> So what would you suggest?

Hmmm... Well I guess your proposal makes sense :-) Aka. having a
documented ABC, and then a private base implementation.

Otherwise, you can also document the methods without saying precisely to
which class they belong, which I started doing on asyncio, but
apparently it confuses some people.

LGTM, although I wish that you had time to implement the comment "TODO: Subclasses can probably optimize this even further" rather than just shuffling the existing code around. :-)

> Guido van Rossum added the comment:
>
> LGTM, although I wish that you had time to implement the comment "TODO: Subclasses can probably optimize this even further" rather than just shuffling the existing code around. :-)

I will look a this, but I rather take care of the API than
implementation details :-)
Also, I'm not sure that modify() is actually a performance-critical
code-path: do you have any semi-realistic asyncio-based benchmark I
could use?

> I think that's a cleaner design.

For common containers like Sequence or Mapping, ABC are useful. But I don't expect new implementations of BaseSelector. Is it just for purity? :-p

(I'm not opposed to the change, I'm just asking.)

In those protocols where client and server exchange a lot of commands and responses in a single session, such as FTP, modify() is going to be called many times.
I don't have actual numbers but I remember that using epoll.modify() was one of those relatively small optimizations which all put together led to current pyftpdlib performances. For poll() and epoll() pollers I see no reason not to use the specialized modify().

That aside, what actually *does* make a big difference and it is very important is to "unregister" a fd for writing (EVENT_WRITE) when there's no more data to send, and that's something that should be done at a higher level (transport.write() and anywhere else some data is sent).

New changeset f48f302f54aa by Charles-François Natali in branch 'default':
Issue #19842: Refactor BaseSelector to make it an actual usable ABC.
http://hg.python.org/cpython/rev/f48f302f54aa

> STINNER Victor added the comment:
>
>> I think that's a cleaner design.
>
> For common containers like Sequence or Mapping, ABC are useful. But I don't expect new implementations of BaseSelector. Is it just for purity? :-p

There are already implementations in the test suite, but the main
reason is for documentation purposes (i.e. to document clearly which
methods are available, and which are optional).
IMO, ABC make documentation more clear, and also make it easier for
third-party implementations.

I don't have a strong opinion on this, my real concern is really just
the documentation, so if you can propose a documentation patch that
makes things clear, I'm perfectly fine with removing BaseSelector from
the API :-)

Do I understand right that BaseSelector.register means something else entirely than ABC.register?  Is it a concern?

Well, registration is a very common pattern and you can't really require everyone to use an inferior word just because ABCMeta uses it.

There's a simple work-around:

abc.ABCMeta.register(selectors.BaseSelector, C)

This is done AFAICT.