Skip to content

gh-102757: fix function signature mismatch for functools.reduce between code and documentation #102759

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

Merged
merged 13 commits into from
Sep 18, 2023

Conversation

XuehaiPan
Copy link
Contributor

@XuehaiPan XuehaiPan commented Mar 16, 2023

  • Align function signature for functools.reduce in Python implementation and documentation with the C implementation.
  • Add / to mark all arguments are positional only.

@rhettinger rhettinger self-assigned this Mar 16, 2023
@rhettinger rhettinger removed their assignment Mar 17, 2023
@XuehaiPan XuehaiPan requested review from sobolevn and removed request for rhettinger March 17, 2023 11:33
@arhadthedev arhadthedev added docs Documentation in the Doc dir extension-modules C modules in the Modules dir labels Mar 22, 2023
@XuehaiPan
Copy link
Contributor Author

May also need to backport to Python 3.10 and 3.11. This documentation issue is causing real-world problems. Such as:

https://github.com/google/jax/blob/1703f096b5b7b66a1c97726c1c89ffdfbbfaaa22/jax/_src/tree_util.py#L274-L280

@arhadthedev
Copy link
Member

@rhettinger (as a functools expert)

Copy link
Member

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It makes sense for the CPython
documentation and docstring of functools.reduce to align with the implementation that is always used in CPython.

@XuehaiPan
Copy link
Contributor Author

Gentle ping for this. Any updates?

@carljm carljm merged commit 74f315e into python:main Sep 18, 2023
@XuehaiPan XuehaiPan deleted the functools.reduce-docs branch September 18, 2023 16:43
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 RHEL7 3.x has failed when building commit 74f315e.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/15/builds/5692) and take a look at the build logs.
  4. Check if the failure is related to this commit (74f315e) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/15/builds/5692

Failed tests:

  • test_socket

Failed subtests:

  • test_dual_stack_client_v4 - test.test_socket.CreateServerFunctionalTest.test_dual_stack_client_v4

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-x86_64/build/Lib/test/test_socket.py", line 6771, in test_dual_stack_client_v4
    with socket.create_server(("", port), family=socket.AF_INET6,
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-x86_64/build/Lib/socket.py", line 935, in create_server
    raise error(err.errno, msg) from None
OSError: [Errno 98] Address already in use (while attempting to bind on address ('', 57716))

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 RHEL8 LTO + PGO 3.x has failed when building commit 74f315e.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/568/builds/4853) and take a look at the build logs.
  4. Check if the failure is related to this commit (74f315e) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/568/builds/4853

Failed tests:

  • test.test_asyncio.test_unix_events

Failed subtests:

  • test_fork_signal_handling - test.test_asyncio.test_unix_events.TestFork.test_fork_signal_handling

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto-pgo/build/Lib/unittest/async_case.py", line 90, in _callTestMethod
    if self._callMaybeAsync(method) is not None:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto-pgo/build/Lib/unittest/async_case.py", line 117, in _callMaybeAsync
    return self._asyncioTestContext.run(func, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto-pgo/build/Lib/test/support/hashlib_helper.py", line 49, in wrapper
    return func_or_class(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto-pgo/build/Lib/test/test_asyncio/test_unix_events.py", line 1937, in test_fork_signal_handling
    self.assertTrue(child_handled.is_set())
AssertionError: False is not true

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 RHEL8 LTO 3.x has failed when building commit 74f315e.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/64/builds/5081) and take a look at the build logs.
  4. Check if the failure is related to this commit (74f315e) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/64/builds/5081

Failed tests:

  • test.test_asyncio.test_unix_events

Failed subtests:

  • test_fork_signal_handling - test.test_asyncio.test_unix_events.TestFork.test_fork_signal_handling

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto/build/Lib/unittest/async_case.py", line 90, in _callTestMethod
    if self._callMaybeAsync(method) is not None:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto/build/Lib/unittest/async_case.py", line 117, in _callMaybeAsync
    return self._asyncioTestContext.run(func, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto/build/Lib/test/support/hashlib_helper.py", line 49, in wrapper
    return func_or_class(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-x86_64.lto/build/Lib/test/test_asyncio/test_unix_events.py", line 1937, in test_fork_signal_handling
    self.assertTrue(child_handled.is_set())
AssertionError: False is not true

@AA-Turner
Copy link
Member

@carljm should we backport the docs part of this PR?

@carljm
Copy link
Member

carljm commented Sep 20, 2023

@AA-Turner Sure, we can. I don't think it's important, but I guess it's possible it could reduce conflict in some other backport in future.

csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Dec 25, 2023
…116398)

The `initial` argument in `functools.reduce` can be `None`.

```python
initial_missing = object()

def reduce(function, iterable, initial=initial_missing, /):
    it = iter(iterable)
    if initial is initial_missing:
        value = next(it)
    else:
        value = initial
    for element in it:
        value = function(value, element)
    return value
```

Reference:

- python/cpython#102759

Pull Request resolved: #116398
Approved by: https://github.com/Skylion007
facebook-github-bot pushed a commit to pytorch/benchmark that referenced this pull request Jan 2, 2024
Summary:
The `initial` argument in `functools.reduce` can be `None`.

```python
initial_missing = object()

def reduce(function, iterable, initial=initial_missing, /):
    it = iter(iterable)
    if initial is initial_missing:
        value = next(it)
    else:
        value = initial
    for element in it:
        value = function(value, element)
    return value
```

Reference:

- python/cpython#102759

X-link: pytorch/pytorch#116398
Approved by: https://github.com/Skylion007

Reviewed By: izaitsevfb, jeanschmidt

Differential Revision: D52424773

fbshipit-source-id: 36d3a6f5f5786350fe8c325773deb135ccbd48e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir extension-modules C modules in the Modules dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants