Behavior of the min/max with key=None

[Amper]

BPO	34149
Nosy	@rhettinger, @terryjreedy, @serhiy-storchaka, @Amper
PRs	bpo-34149: Behavior of the min/max with key=None #8328

^{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 2018-07-24.03:59:33.945>
created_at = <Date 2018-07-18.16:06:09.916>
labels = ['3.8', 'type-feature', 'library']
title = 'Behavior of the min/max with key=None'
updated_at = <Date 2018-07-24.03:59:33.943>
user = 'https://github.com/Amper'

bugs.python.org fields:

activity = <Date 2018-07-24.03:59:33.943>
actor = 'rhettinger'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-24.03:59:33.945>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2018-07-18.16:06:09.916>
creator = 'amper'
dependencies = []
files = []
hgrepos = []
issue_num = 34149
keywords = ['patch']
message_count = 12.0
messages = ['321891', '321907', '321922', '321923', '321930', '321934', '321937', '322061', '322062', '322067', '322276', '322277']
nosy_count = 5.0
nosy_names = ['rhettinger', 'terry.reedy', 'bethard', 'serhiy.storchaka', 'amper']
pr_nums = ['8328']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue34149'
versions = ['Python 3.8']

[Amper]

I was faced with the fact that the behavior of the functions "min"/"max" and "sorted" is a little different.

For example, this code works fine:

    >>> sorted([3, 2, 1], key=None)
    [1, 2, 3]

But the same example for "min" and "max" doesn't work:

    >>> min([3, 2, 1], key=None)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: 'NoneType' object is not callable

That is why the heapq library has this code:

...
def nsmallest(n, iterable, key=None):
    ...
        if key is None:
            result = min(it, default=sentinel)
        else:
            result = min(it, default=sentinel, key=key)
    ...

At the same time, there are many places where such checks are not performed for the "sorted" function. I think the behavior of the "min" / "max" / "sorted" functions should be unified. That is, they should work as if "None" is the default value for "key".

[serhiy-storchaka]

Accepting None has a drawback. It can hide a bug when the key function unexpectedly becomes None.

[rhettinger]

+0 Probably users will never care about this but I don't see a downside beyond the small API churn.

[serhiy-storchaka]

Whouldn't be better to add operator.identity and use it as the default value instead of None?

[rhettinger]

Whouldn't be better to add operator.identity and use it as the default value instead of None?

No, that would incur overhead that currently isn't present.

[serhiy-storchaka]

Accepting None makes the typing model more complex. Instead of just a callable functions accept callable-or-none. It terms of annotations, it is Union[Callable[[Any], Any], None] instead of just Callable[[Any], Any].

[rhettinger]

Serhiy, feel free to reject this PR.

[terryjreedy]

In 2.x, map(None, 'abc', 'zyz') == [('a', 'z'), ('b', 'y'), ('c', 'z')], but with the addition of zip, so that zip('abc', 'xyz') has the same result, we deprecated that use of None to mean identity function.

For python-coded functions, a default is needed to make a keyword-only argument optional, and preferred over use of *args for making positional arguments optional. Unless I am missing something, a function can special-case 'key is identity', to avoid overhead, just as well as it can special-case 'key is None'. So rather than extend 'key=None', to me a kludge, I would rather replace it with 'key=identity'. Both can be accepted during a deprecation period.

For instance, after adding identity,

def nsmallest(n, iterable, key=identity):
    ...
        if key is identity or key is None:  # key in (identity, None)
            result = min(it, default=sentinel)
    ...

Since no one need ever write key=None, explicit passing should be rare. It seems to me that the main reason for the type declaration of key to include None is so that the def statement itself passes a consistency check with the None default. Once that is changed, most people should be able to use a simple callable declaration. I am considering this for python-ideas.

Since the weekly issues list came out just 10 hours ago, I will not close this yet, but I will if still open in couple of days and no coredev objections.

[terryjreedy]

FWIW, the nsmallest/largest key param was added Dec 2, 2004, before keyword-only parameters.
4901a1f

[rhettinger]

Repeating my comment on the Github PR, "I'm persuaded by your idea that min(), max(), nsmallest(), nlargest(), sorted(), and itertools.groupby() should ideally have the same API for key functions. Syncing of those APIs would be a minor improvement. As Serhiy pointed out, this does complicate the type signature a bit, but that is of small concern giving that the other four functions listed have already gone down this path."

[rhettinger]

New changeset e22072f by Raymond Hettinger (Alexander Marshalov) in branch 'master':
bpo-34149: Behavior of the min/max with key=None (GH-8328)
e22072f

[rhettinger]

Alexander, thanks for the suggestion and patch.