Incorrect __text_signature__ for sorted

[eriknw]

BPO	26729
Nosy	@rhettinger, @ncoghlan, @vadmium, @serhiy-storchaka, @eriknw
Dependencies	bpo-26282: Add support for partial keyword arguments in extension functions
Files	sorted_1.diff: Update the docstring of sorted builtin sorted_2.patch: flexible call signature; sorted matches original text_signature sorted_3.patch: Correct text_signature for sorted. Behavior unchanged.

^{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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-01-23.10:31:57.824>
created_at = <Date 2016-04-10.17:54:40.518>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'Incorrect __text_signature__ for sorted'
updated_at = <Date 2017-01-23.10:31:57.823>
user = 'https://github.com/eriknw'

bugs.python.org fields:

activity = <Date 2017-01-23.10:31:57.823>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-01-23.10:31:57.824>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2016-04-10.17:54:40.518>
creator = 'eriknw'
dependencies = ['26282']
files = ['42422', '42441', '42444']
hgrepos = []
issue_num = 26729
keywords = ['patch']
message_count = 13.0
messages = ['263145', '263155', '263222', '263227', '263230', '263231', '263238', '263244', '263246', '263280', '285923', '286063', '286069']
nosy_count = 6.0
nosy_names = ['rhettinger', 'ncoghlan', 'python-dev', 'martin.panter', 'serhiy.storchaka', 'eriknw']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26729'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[eriknw]

The first argument to sorted is positional-only, so the text signature should be:

sorted($module, iterable, /, key=None, reverse=False)

instead of

sorted($module, iterable, key=None, reverse=False)

To reproduce the issue, attempt to use "iterable" as a keyword argument:

>> import inspect
>> sig = inspect.signature(sorted)
>> sig.bind(iterable=[]) # should raise, but doesn't
>> sorted(iterable=[]) # raises TypeError

[vadmium]

This is a strange case. It looks like “iterable” is half-supported as a keyword argument. So Silent Ghost’s patch fixes the signature, but the code still tries to accept keyword arguments:

>>> sorted(iterable=None)
TypeError: 'NoneType' object is not iterable
>>> sorted(iterable=())
TypeError: 'iterable' is an invalid keyword argument for this function

The problem is that sorted() blindly passes the keyword arguments to list.sort(). I guess we could delete "iterable" from them, but maybe it is not worth the trouble.

[eriknw]

Interesting observation, Martin.

Upon further consideration, the call signature for sorted really is quite odd. It doesn't behave like any other builtin function. Currently, "iterable" is positional-only, and "key=" and "reverse=" are keyword only. I would only expect such behavior for functions with variadic *args.

I uploaded a new patch so that the call signature matches the original __text_signature__. This means "iterable" may be given as a keyword argument, and "key" and "reverse" may be given as positional arguments.

I added tests for the new behavior, and all tests pass for me.

[rhettinger]

I don't think we should start down the path of changing APIs just to accommodate weakness in the generation of the text signature.

We don't want to encourage unreadable oddities like sorted(reverse=False, iterable=source). Best readability comes from the required positional argument for the iterable. Please don't create a new and unnecessary keyword argument. The current API is intentional.

[eriknw]

That's a fair and valid point, Raymond. "sorted_2.patch" was submitted for consideration. Either __text_signature__ is wrong, or the call argument handling is wrong. One should be fixed.

Having a flexible call signature as if sorted were a user-defined function, such as "def sorted(iterable, key=None, reverse=False):", does allow for programmatic use of the introspected signature. Here, using "iterable=" as a keyword can be convenient.

"sorted_1.diff" is wrong. To match the existing call signature, __text_signature__ should be:

sorted($module, iterable, /, *, key=None, reverse=False)

I don't know any other builtin with a signature like this. Such a signature may be a point of confusion for people learning Python ("what are those funny symbols?!"), who often encounter sorted early on.

[serhiy-storchaka]

See bpo-26282.

[ncoghlan]

+1 for Serhiy's suggestion of enhancing the C API to specifically handle these cases.

[vadmium]

I didn’t realize about the keyword-only parameters. This is inherited from list.sort(). The signature can be corrected in 3.5.

There is also bpo-21314 about documenting the slash notation for signatures (it comes from PEP-457).

[eriknw]

sorted_3.patch corrects the __text_signature__. Behavior of sorted is unchanged.

>>> def raises(err, lamda):
...     try:
...         lamda()
...         return False
...     except err:
...         return True
...
>>> import inspect
>>> sig = inspect.signature(sorted) 
>>> # `iterable` is positional-only
>>> assert raises(TypeError, lambda: sorted(iterable=[]))
>>> assert raises(TypeError, lambda: sig.bind(iterable=[]))
>>> # `key` and `reverse` are keyword-only
>>> assert raises(TypeError, lambda: sorted([], lambda x: x))
>>> assert raises(TypeError, lambda: sig.bind([], lambda x: x))

[vadmium]

Patch 3 looks okay to me.

[serhiy-storchaka]

See also bpo-29327 and bpo-29331.

sorted_3.patch LGTM.

[rhettinger]

Serhiy, do you want to apply this?

[python-dev]

New changeset c0a9fb3e19b9 by Serhiy Storchaka in branch '3.5':
Issue bpo-26729: Fixed __text_signature__ for sorted().
https://hg.python.org/cpython/rev/c0a9fb3e19b9

New changeset fcb19fb42058 by Serhiy Storchaka in branch '3.6':
Issue bpo-26729: Fixed __text_signature__ for sorted().
https://hg.python.org/cpython/rev/fcb19fb42058

New changeset 4ad195d9e184 by Serhiy Storchaka in branch 'default':
Issue bpo-26729: Fixed __text_signature__ for sorted().
https://hg.python.org/cpython/rev/4ad195d9e184