Skip to content

Conversation

@pmhahn
Copy link
Contributor

@pmhahn pmhahn commented Oct 8, 2020

instead of bool, as there are many contradicting signatures:

https://docs.python.org/3/library/socket.html#socket.socket.setblocking
names the arguments flags and uses false in the descriptive text,
but uses bool in the given examples.

https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L149
uses bool in the module documentation, but the internal wrapper in
https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L658
uses int, which is called by the public function in
https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L2783
using PyLong_AsLong() to get an int.

So int is correct as bool is a subtype of it and gets promoted to it
automatically.

Signed-off-by: Philipp Hahn hahn@univention.de

instead of `bool`, as there are many contradicting signatures:

<https://docs.python.org/3/library/socket.html#socket.socket.setblocking>
names the arguments `flags` and uses `false` in the descriptive text,
but uses `bool` in the given examples.

<https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L149>
uses `bool` in the module documentation, but the internal wrapper in
<https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L658>
uses `int` which is called by the public function in
<https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L2783>
using `PyLong_AsLong()` to get the value of `flags` as an `int`.

So `int` is correct as `bool` is a subtype of it and gets promoted to it
automatically.

Signed-off-by: Philipp Hahn <hahn@univention.de>
@srittau
Copy link
Collaborator

srittau commented Oct 8, 2020

I'm -1 on this change. The argument is clearly used as a boolean value. With the same justification we could change nearly all bool arguments in typeshed to int, as True == 1 and False == 0.

@pmhahn pmhahn deleted the socket branch October 28, 2020 17:13
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

Successfully merging this pull request may close these issues.

3 participants