Add pure Python fallback for functools.reduce

[stevendaprano]

BPO	32321
Nosy	@rhettinger, @ncoghlan, @vstinner, @stevendaprano, @asvetlov
PRs	bpo-32321: reduce is guaranteed to be implemented in _functools, so no need to use ImportError guard #4949 bpo-32321: Add pure Python fallback for functools.reduce #8548 bpo-32321: Add pure Python fallback for functools.reduce #9884 [3.7] bpo-32321: Add pure Python fallback for functools.reduce (GH-8548) #10092

^{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/ncoghlan'
closed_at = <Date 2018-10-25.14:10:34.470>
created_at = <Date 2017-12-14.13:26:34.576>
labels = ['easy', '3.8', 'library']
title = 'Add pure Python fallback for functools.reduce'
updated_at = <Date 2018-10-25.15:01:16.818>
user = 'https://github.com/stevendaprano'

bugs.python.org fields:

activity = <Date 2018-10-25.15:01:16.818>
actor = 'vstinner'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2018-10-25.14:10:34.470>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2017-12-14.13:26:34.576>
creator = 'steven.daprano'
dependencies = []
files = []
hgrepos = []
issue_num = 32321
keywords = ['patch', 'easy']
message_count = 6.0
messages = ['308297', '308856', '310914', '328437', '328438', '328446']
nosy_count = 5.0
nosy_names = ['rhettinger', 'ncoghlan', 'vstinner', 'steven.daprano', 'asvetlov']
pr_nums = ['4949', '8548', '9884', '10092']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue32321'
versions = ['Python 3.8']

[stevendaprano]

The functools module imports reduce from _functools, using a guard in case it is not present:

try:
from _functools import reduce
except ImportError:
pass

However, the documentation says nothing about reduce being optional, and it is unconditionally included in the module __all__.

If reduce is guaranteed to be implemented in _functools, then the guard is redundant and should be removed. Otherwise, a pure python fallback should be added.

(The docs for reduce include a pure Python equivalent which might be sufficient.)

[asvetlov]

See discussion in https://bugs.python.org/issue12428 (especially the last message)

[ncoghlan]

Reviewing the code and the CI test failures on the PR, the trick here is that functools isn't actually *using* functools.reduce, it's just re-exporting it if it's defined.

So if you block importing of "_functools" (which the test suite does in order to test the pure Python fallbacks), then the *only* consequence is that "functools.reduce" will be missing - the module will otherwise be fine.

This isn't at all clear when reading the code though, so I think the simplest resolution here would be to add a comment to the fallback path that says "If _functools.reduce is missing, then functools.reduce will also be missing, but the module will otherwise work".

Alternatively, we could add a fallback implementation based on the recipe in the docs, and adjust the test suite to actually run the reduce tests against the py_functools variant: https://docs.python.org/3/library/functools.html#functools.reduce

[vstinner]

New changeset e25d5fc by Victor Stinner (madman-bob) in branch 'master':
bpo-32321: Add pure Python fallback for functools.reduce (GH-8548)
e25d5fc

[vstinner]

I just merged PR 8548. See the PR to the discussion. IMHO it's a nice enhancement to have a pure Python implementation:
#8548 (comment)
(and the PR has been approved by 2 other core devs ;-))

But I'm against adding it to Python 3.7 as well, I rejected the backport: PR 10092.

I proposed the author to write another PR to refactor test_functools.py, but I don't think that it's worth it to keep this issue open just for that:
#8548 (comment)

[vstinner]

However, the documentation says nothing about reduce being optional, and it is unconditionally included in the module __all__.

Oh, about this specific issue: maybe test___all__ should be fixed to test functools.py with _functools blocked? As done by test_functools.py? But this is a wider change and I'm not sure that it's doable :-( (especially in a generic way!) The PEP-399 requires the same API for the C and the Python implementations.