Issue32321
Created on 2017-12-14 13:26 by steven.daprano, last changed 2022-04-11 14:58 by admin. 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) *  |
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) *  |
Date: 2017-12-21 06:22 |
See discussion in https://bugs.python.org/issue12428 (especially the last message) |
|
|
msg310914 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
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) *  |
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 |
2022-04-11 14:58:55 |
admin |
set |
github: 76502 |
2018-10-25 15:01:16 |
vstinner |
set |
messages: + |
2018-10-25 14:10:52 |
vstinner |
set |
title: functools.reduce has a redundant guard or needs a pure Python fallback -> Add pure Python fallback for functools.reduce |
2018-10-25 14:10:34 |
vstinner |
set |
status: open -> closedversions: + Python 3.8, - Python 3.7messages: + resolution: fixedstage: patch review -> resolved |
2018-10-25 14:02:36 |
miss-islington |
set |
pull_requests: + <pull%5Frequest9425> |
2018-10-25 14:02:17 |
vstinner |
set |
nosy: + vstinnermessages: + |
2018-10-25 14:01:07 |
serhiy.storchaka |
set |
nosy: + rhettinger |
2018-10-14 23:55:57 |
bradengroom |
set |
pull_requests: + <pull%5Frequest9246> |
2018-07-29 12:37:41 |
madman bob |
set |
pull_requests: + <pull%5Frequest8064> |
2018-01-28 04:33:30 |
ncoghlan |
set |
messages: + |
2017-12-21 06:22:33 |
asvetlov |
set |
nosy: + asvetlovmessages: + |
2017-12-20 19:55:10 |
thatiparthy |
set |
keywords: + patchstage: patch reviewpull_requests: + <pull%5Frequest4841> |
2017-12-16 09:14:51 |
rhettinger |
set |
assignee: ncoghlannosy: + ncoghlan |
2017-12-14 13:26:34 |
steven.daprano |
create |
|