Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pure Python fallback for functools.reduce #76502

Closed
stevendaprano opened this issue Dec 14, 2017 · 6 comments
Closed

Add pure Python fallback for functools.reduce #76502

stevendaprano opened this issue Dec 14, 2017 · 6 comments
Assignees
Labels
3.8 only security fixes easy stdlib Python modules in the Lib dir

Comments

@stevendaprano
Copy link
Member

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
    Copy link
    Member Author

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

    @stevendaprano stevendaprano added 3.7 (EOL) end of life stdlib Python modules in the Lib dir easy labels Dec 14, 2017
    @asvetlov
    Copy link
    Contributor

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

    @ncoghlan
    Copy link
    Contributor

    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
    Copy link
    Member

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

    @vstinner
    Copy link
    Member

    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 vstinner added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Oct 25, 2018
    @vstinner vstinner changed the title functools.reduce has a redundant guard or needs a pure Python fallback Add pure Python fallback for functools.reduce Oct 25, 2018
    @vstinner
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes easy stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants