kqueue.control() documentation and implementation mismatch

[abadger]

BPO	34369
Nosy	@taleinat, @abadger, @berkerpeksag, @vadmium, @miss-islington, @tirkarthi
PRs	bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only #9499 [3.8] bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499) #14704 [3.7] bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499) #14705 [2.7] bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499) #14706

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-07-11.14:19:17.966>
created_at = <Date 2018-08-10.06:20:16.853>
labels = ['3.8', 'type-bug', '3.7', '3.9', 'docs']
title = 'kqueue.control() documentation and implementation mismatch'
updated_at = <Date 2019-07-11.14:19:17.966>
user = 'https://github.com/abadger'

bugs.python.org fields:

activity = <Date 2019-07-11.14:19:17.966>
actor = 'taleinat'
assignee = 'docs@python'
closed = True
closed_date = <Date 2019-07-11.14:19:17.966>
closer = 'taleinat'
components = ['Documentation']
creation = <Date 2018-08-10.06:20:16.853>
creator = 'a.badger'
dependencies = []
files = []
hgrepos = []
issue_num = 34369
keywords = ['patch']
message_count = 13.0
messages = ['323357', '323366', '323370', '323426', '323435', '323438', '323439', '326616', '326620', '347676', '347678', '347679', '347680']
nosy_count = 7.0
nosy_names = ['taleinat', 'a.badger', 'docs@python', 'berker.peksag', 'martin.panter', 'miss-islington', 'xtreak']
pr_nums = ['9499', '14704', '14705', '14706']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue34369'
versions = ['Python 2.7', 'Python 3.7', 'Python 3.8', 'Python 3.9']

[abadger]

The current kqueue documentation specifies that timeout is a keyword argument but it can only be passed as a positional argument right now:

>>> import select
>>> ko = select.kqueue()
>>> ko.control([1], 0, timeout=10)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: control() takes no keyword arguments
>>> help(ko.control)
Help on built-in function control:

control(...) method of select.kqueue instance
control(changelist, max_events[, timeout=None]) -> eventlist

Calls the kernel kevent function.
- changelist must be an iterable of kevent objects describing the changes
  to be made to the kernel's watch list or None.
- max_events lets you specify the maximum number of events that the
  kernel will return.
- timeout is the maximum time to wait in seconds, or else None,
  to wait forever. timeout accepts floats for smaller timeouts, too.

This may be related to https://bugs.python.org/issue3852 in which the max_events argument used to be documented as optional but the code made it mandatory.

[berkerpeksag]

This is probably a regression from the Argument Clinic conversion.

Another docstring mismatches:

select(rlist, wlist, xlist, timeout=None, /)
    Wait until one or more file descriptors are ready for some kind of I/O.

>>> select.select(timeout=0.1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: select() takes no keyword arguments

poll(timeout=None, /) method of select.poll instance
Polls the set of registered file descriptors.

>>> select.poll().poll(timeout=0.1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: poll() takes no keyword arguments

[abadger]

I don't believe (kqueue.control at least) is a regression from Argument Clinic. Both the documentation and the behaviour are the same in Python-2.7.

[vadmium]

I think this was an attempt to specify a positional-only parameter (by using square brackets), and include a default value in the signature. The usual approach in this situation is to use square brackets, but only mention the default value in the text:

control(changelist, max_events[, timeout])
. . .
The default for "timeout" is None, to wait forever.

In bpo-13386 Ezio suggested against showing both square brackets and a default value, and I agree.

Berker: your "select" and "poll" cases all include the PEP-457 slash "/" notation, which was supposed to indicate positional-only parameters. See bpo-21314 about explaining this notation in the documentation somewhere.

[taleinat]

In Python 3.8 this will have already been fixed (thanks to the recent conversion of the select module to use Argument Clinic); see below output from a recent build from the master branch.

Given that, is this worth fixing on 2.7 and <3.8?

Help on built-in function control:

control(changelist, maxevents, timeout=None, /) method of select.kqueue instance
Calls the kernel kevent function.

changelist
  Must be an iterable of kevent objects describing the changes to be made
  to the kernel's watch list or None.
maxevents
  The maximum number of events that the kernel will return.
timeout
  The maximum time to wait in seconds, or else None to wait forever.
  This accepts floats for smaller timeouts, too.

[vadmium]

Even in 3.8, the main documentation is not fixed: https://docs.python.org/dev/library/select.html#kqueue-objects

[taleinat]

We can just remove the "=None" in the docs for select.kqueue.control() to conform with the rest of that doc.

We don't use the PEP-457 slash notation in the docs for select (or perhaps at all?).

[vadmium]

I think removing all mention of “None” is a step too far. The “devpoll”, “epoll”, and “poll” documentation all say that “None” is acceptable for the timeout. Only the “select” function doesn’t say this.

What about adding to the text:

• “timeout” in seconds (floats possible); the default is “None”, to wait forever

[taleinat]

I agree Martin, I hadn't noticed that the docs don't otherwise mention the default behavior WRT timeout.

I've added a description as per your suggestion to the PR.

[taleinat]

New changeset 79042ac by Tal Einat in branch 'master':
bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499)
79042ac

[miss-islington]

New changeset dc0b6af by Miss Islington (bot) in branch '3.8':
bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499)
dc0b6af

[taleinat]

New changeset d3747fd by Tal Einat (Miss Islington (bot)) in branch '3.7':
bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499)
d3747fd

[taleinat]

New changeset 46c2eff by Tal Einat (Miss Islington (bot)) in branch '2.7':
bpo-34369: make kqueue.control() docs better reflect that timeout is positional-only (GH-9499)
46c2eff