Incorrect __text_signature__ for sorted

@rhettinger

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']

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

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.

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.

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.

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.

See bpo-26282.

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

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).

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))

Patch 3 looks okay to me.

Navigation Menu

Repository navigation

Incorrect __text_signature__ for sorted #70916

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Incorrect __text_signature__ for sorted #70916

Description

Activity

eriknw commented on Apr 10, 2016

vadmium commented on Apr 10, 2016

eriknw commented on Apr 12, 2016

rhettinger commented on Apr 12, 2016

eriknw commented on Apr 12, 2016

serhiy-storchaka commented on Apr 12, 2016

ncoghlan commented on Apr 12, 2016

vadmium commented on Apr 12, 2016

eriknw commented on Apr 12, 2016

vadmium commented on Apr 12, 2016

serhiy-storchaka commented on Jan 20, 2017

rhettinger commented on Jan 23, 2017

python-dev commented on Jan 23, 2017

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions