classification
Title: Add pure Python fallback for functools.reduce
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: asvetlov, ncoghlan, rhettinger, steven.daprano, vstinner
Priority: normal Keywords: easy, patch

Created on 2017-12-14 13:26 by steven.daprano, last changed 2018-10-25 15:01 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4949 closed thatiparthy, 2017-12-20 19:55
PR 8548 merged madman bob, 2018-07-29 12:37
PR 9884 closed bradengroom, 2018-10-14 23:55
PR 10092 closed miss-islington, 2018-10-25 14:02
Messages (6)
msg308297 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2017-12-14 13:26
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.)
msg308856 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2017-12-21 06:22
See discussion in https://bugs.python.org/issue12428 (especially the last message)
msg310914 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-01-28 04:33
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
msg328437 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-25 14:02
New changeset e25d5fc18e6c4b0062cd71b2eb1fd2d5eb5e2d3d by Victor Stinner (madman-bob) in branch 'master':
bpo-32321: Add pure Python fallback for functools.reduce (GH-8548)
https://github.com/python/cpython/commit/e25d5fc18e6c4b0062cd71b2eb1fd2d5eb5e2d3d
msg328438 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-25 14:10
I just merged PR 8548. See the PR to the discussion. IMHO it's a nice enhancement to have a pure Python implementation:
https://github.com/python/cpython/pull/8548#issuecomment-433063178
(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:
https://github.com/python/cpython/pull/8548#issuecomment-433065931
msg328446 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-25 15:01
> 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.
History
Date User Action Args
2018-10-25 15:01:16vstinnersetmessages: + msg328446
2018-10-25 14:10:52vstinnersettitle: functools.reduce has a redundant guard or needs a pure Python fallback -> Add pure Python fallback for functools.reduce
2018-10-25 14:10:34vstinnersetstatus: open -> closed
versions: + Python 3.8, - Python 3.7
messages: + msg328438

resolution: fixed
stage: patch review -> resolved
2018-10-25 14:02:36miss-islingtonsetpull_requests: + pull_request9425
2018-10-25 14:02:17vstinnersetnosy: + vstinner
messages: + msg328437
2018-10-25 14:01:07serhiy.storchakasetnosy: + rhettinger
2018-10-14 23:55:57bradengroomsetpull_requests: + pull_request9246
2018-07-29 12:37:41madman bobsetpull_requests: + pull_request8064
2018-01-28 04:33:30ncoghlansetmessages: + msg310914
2017-12-21 06:22:33asvetlovsetnosy: + asvetlov
messages: + msg308856
2017-12-20 19:55:10thatiparthysetkeywords: + patch
stage: patch review
pull_requests: + pull_request4841
2017-12-16 09:14:51rhettingersetassignee: ncoghlan

nosy: + ncoghlan
2017-12-14 13:26:34steven.dapranocreate