This adds a keys() method to selectors, to return all the registered keys.
It's useful, because one often needs to loop until all registered file objects have been unregistered e.g. (inspired from subprocess, see #18923):

while selector.keys():
   for key, events in selector.select():
       # process events

also, it can be useful if you want e.g. the list of all currently logged users, etc. It avoids having to maintain a separate data-structure tracking registered file objects.

The patch attached returns a new set upon each call: another way to handle this would be to just return the dictionary's .values() view: it would be faster and use less memory, but it's immutable and also this would prevent the user from doing:
for key in selectior.keys():
   if somecondition:
       selector.unregister(key.fileobj)

(since you'll get "dictonary changed size during iteration").

If you want to unregister while iterating on .keys(), just copy .keys(), as
I do when removing items from a list or a dict while iterating on it.

Indeed it's easy enough to call list() or set() on the result if you need it.

On the other hand, the problem with returning a dict view is that it makes the return type dependent on an implementation detail. Returning a simple iterator would be great, except that you can't as easily test it for emptiness.

So perhaps two methods are needed:
- a keys() method returning an iterator
- a has_keys() method returning a boolean

Using .keys() to test if the selector is empty is surprising. To test if a
str, list, tuple, dict, set, ..., is empty: if container: <not empty> is
enough.

Or sometimes I write if len(container): ...

Selector has no length?

> If you want to unregister while iterating on .keys(), just copy .keys(), as
> I do when removing items from a list or a dict while iterating on it.

Of course, but as noted by Antoine, it makes the return type dependent of an
implementation detail (I don't think there's any other place in the stdlib
where a dict view is returned).

> Using .keys() to test if the selector is empty is surprising. To test if a
> str, list, tuple, dict, set, ..., is empty: if container: <not empty> is
> enough.
>
> Or sometimes I write if len(container): ...
>
> Selector has no length?

No. For example, select.epoll objects don't have a length either.
I'm not sure whether selectors have a natural length: if we added a __len__,
we should also add __iter__ and IMO a selector isn't a container.

Returning all the keys is a generic method that can be useful besides checking
that the selector has registered keys.

> So perhaps two methods are needed:
> - a keys() method returning an iterator
> - a has_keys() method returning a boolean

Sounds a bit overkill, no?
keys() is definitely usful, and I'm not sure that it'll actually be a
performance
bottleneck in practice.

BaseSelector.register(fd) raises a KeyError if fd is already registered, which means that any selector must know the list of all registered FDs. For EpollSelector, the list may be inconsistent *if* the epoll is object is modified externally, but it's really a corner case.

"while selector: ..." is a nice (common?) pattern, it is used by subprocess.communicate() at least.

> I'm not sure whether selectors have a natural length

len(self._fd_to_key) is the natural length. bool(selector) uses selector.__length__() is defined, right?


What do you think of having some mapping methods?

- iter(selector), selector.keys(): : iter(self._fd_to_key), iterator on file descriptor numbers (int)
- selector.values(): self._fd_to_key.values(), iterate on SelectorKey objects
- selector.items(): self._fd_to_key.items(), iterator on (fd, SelectorKey) tuples

"SelectorKey" name is confusing, it's not really a key as a dictionary dict. SelectorKey.fd *is* the key, Selector.event is not.

"SelectorFile" or "SelectorItem" would be more explicit, no?

By the way, is SelectorKey.fileobj always defined? If not, the documentation is wrong: the attribut should be documented as "Optional", as .data.

> BaseSelector.register(fd) raises a KeyError if fd is already registered, which means that any selector must know the list of all registered FDs. For EpollSelector, the list may be inconsistent *if* the epoll is object is modified externally, but it's really a corner case.

Yes, and there's nothing we can do about it :-(

> What do you think of having some mapping methods?
>
> - iter(selector), selector.keys(): : iter(self._fd_to_key), iterator on file descriptor numbers (int)
> - selector.values(): self._fd_to_key.values(), iterate on SelectorKey objects
> - selector.items(): self._fd_to_key.items(), iterator on (fd, SelectorKey) tuples

I don't know, it makes me uncomfortable treating a selector like
a plain container.

> "SelectorKey" name is confusing, it's not really a key as a dictionary dict. SelectorKey.fd *is* the key, Selector.event is not.
>
> "SelectorFile" or "SelectorItem" would be more explicit, no?

It's more key as "indexing key", or token: it's the interface between the selector and the external world.
The FD "is" indeed the key internally, but that's an implementation detail.

I'd really like to have Guido's feeling regarding the above questions.

> By the way, is SelectorKey.fileobj always defined? If not, the documentation is wrong: the attribut should be documented as "Optional", as .data.

Yes, it's always defined: it's the object passed to register().

2013/10/6 Charles-François Natali <report@bugs.python.org>:
>> BaseSelector.register(fd) raises a KeyError if fd is already registered, which means that any selector must know the list of all registered FDs. For EpollSelector, the list may be inconsistent *if* the epoll is object is modified externally, but it's really a corner case.
>
> Yes, and there's nothing we can do about it :-(

Oh, I just mentioned to corner case to say that it would nice to
expose the length of a selector.

>> What do you think of having some mapping methods?
>>
>> - iter(selector), selector.keys(): : iter(self._fd_to_key), iterator on file descriptor numbers (int)
>> - selector.values(): self._fd_to_key.values(), iterate on SelectorKey objects
>> - selector.items(): self._fd_to_key.items(), iterator on (fd, SelectorKey) tuples
>
> I don't know, it makes me uncomfortable treating a selector like
> a plain container.

I don't know if there is a real use case.

>> By the way, is SelectorKey.fileobj always defined? If not, the documentation is wrong: the attribut should be documented as "Optional", as .data.
>
> Yes, it's always defined: it's the object passed to register().

Oh, selector.register(0).fileobj gives me 0... I didn't know that 0 is
a file object :-) I would expect .fileobj=None and .fd=0.

No time to follow this in detail, but one thing: please do not make the selector appear "false" under *any* circumstances. I've seen too many code write "if foo" where they meant "if foo is not None" and get in trouble because foo wasn't None but happened to have no content. (See e.g. recent issue 19097, although the situation there is even more complicated.) (And for things formally deriving from Container it's a different thing. But that shouldn't be done lightly.)

I think it's useful to be able to get the keys; it would be nice if that was an O(1) operation so you can also check for emptiness without needing a second method. Perhaps the method shouldn't be called keys() to avoid any confusion with subclasses of the Container ABC?

> Perhaps the method shouldn't be called keys() to avoid any confusion with subclasses of the Container ABC?

If you don't want to rename the SelectorKey class, rename the method
to get_keys().

FWIW, I think the "ideal" solution would be for keys() (*) to return a
read-only Mapping implementation, allowing for file object lookup (using
__getitem__) as well as iteration on SelectorKeys (using __iter__) and
fast emptiness checking (using __len__).

(to implement Mapping, you can subclass Mapping and implement
__getitem__, __len__ and __iter__)

(*) or a better name

> FWIW, I think the "ideal" solution would be for keys() (*) to return a
> read-only Mapping implementation, allowing for file object lookup (using
> __getitem__) as well as iteration on SelectorKeys (using __iter__) and
> fast emptiness checking (using __len__).

Actually, you would use values() to iterate on SelectorKeys.

Here is a proof-of-concept patch adding a get_map() method to Selector (and removing get_key()).

> Antoine Pitrou added the comment:
>
> FWIW, I think the "ideal" solution would be for keys() (*) to return a
> read-only Mapping implementation, allowing for file object lookup (using
> __getitem__) as well as iteration on SelectorKeys (using __iter__) and
> fast emptiness checking (using __len__).

Thanks, I think that's a great idea.
I'm attaching a patch updating yours with the following:
- asyncio/test_asyncio update
- selectors' documentation update

IMO, it really offers both a compact, easy to use and performant API.

(Note: the mapping doesn't really have to be recreated upon each
get_map() call and could be kept as a private member, but IMO that's
not a performance issue).

I added some comments to the code review. Please take a look.

(Adding CF's new patch so I can compare and review it.)

LGTM. Commit!

New changeset b0ae96700301 by Charles-François Natali in branch 'default':
Issue #19172: Add a get_map() method to selectors.
http://hg.python.org/cpython/rev/b0ae96700301

Committed.

Antoine, thanks for the idea and patch!

+   .. method:: get_map()
+
+      Return a mapping of file objects to selector keys.

I kind of feel like a walking linter though I think this needs a versionadded tag :)

> Berker Peksag added the comment:
>
> +   .. method:: get_map()
> +
> +      Return a mapping of file objects to selector keys.
>
> I kind of feel like a walking linter though I think this needs a versionadded tag :)

Well, selectors have been added to 3.4 - which hasn't been released
yet - so no versionadded is needed (there's one at the module-level).