Skip to content

Invalid error type in math.factorial sphinx docs/docstring #133904

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

Closed
skirpichev opened this issue May 11, 2025 · 5 comments
Closed

Invalid error type in math.factorial sphinx docs/docstring #133904

skirpichev opened this issue May 11, 2025 · 5 comments
Labels
3.13 bugs and security fixes 3.14 bugs and security fixes docs Documentation in the Doc dir easy

Comments

@skirpichev
Copy link
Member

skirpichev commented May 11, 2025

Documentation says: Raises ValueError if n is not integral or is negative.
Docstring: Raise a ValueError if x is negative or non-integral.

State of art:

>>> math.factorial(1.0)
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    math.factorial(1.0)
    ~~~~~~~~~~~~~~^^^^^
TypeError: 'float' object cannot be interpreted as an integer
>>> math.factorial(-1.0)
Traceback (most recent call last):
  File "<python-input-3>", line 1, in <module>
    math.factorial(-1.0)
    ~~~~~~~~~~~~~~^^^^^^
TypeError: 'float' object cannot be interpreted as an integer
>>> math.factorial(-1)
Traceback (most recent call last):
  File "<python-input-4>", line 1, in <module>
    math.factorial(-1)
    ~~~~~~~~~~~~~~^^^^
ValueError: factorial() not defined for negative values

Documentation should be fixed to state, that TypeError is raised for non-integral arguments.

Or just omit description of exceptions and reduce docs to: "Return factorial of the nonnegative integer n."

Linked PRs

@skirpichev skirpichev added docs Documentation in the Doc dir easy 3.13 bugs and security fixes 3.14 bugs and security fixes labels May 11, 2025
@StanFromIreland
Copy link
Contributor

"Return factorial of the nonnegative integer n."

+1, clear as to what n should be, error should be expected otherwise.

@mkaraev
Copy link
Contributor

mkaraev commented May 11, 2025

I opened PR
#133907

@StanFromIreland
Copy link
Contributor

The pr is automatically linked, you do not need to comment it.

sobolevn added a commit that referenced this issue May 12, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 12, 2025
(cherry picked from commit 27ed645)

Co-authored-by: mkaraev <[email protected]>
Co-authored-by: sobolevn <[email protected]>
sobolevn added a commit that referenced this issue May 12, 2025
…3922)

* [3.13] gh-133904: Fix `math.factorial` documentation (GH-133907)
(cherry picked from commit 27ed645)

Co-authored-by: mkaraev <[email protected]>
Co-authored-by: sobolevn <[email protected]>
sobolevn added a commit that referenced this issue May 12, 2025
…3918)

gh-133904: Fix `math.factorial` documentation (GH-133907)
(cherry picked from commit 27ed645)

Co-authored-by: mkaraev <[email protected]>
Co-authored-by: sobolevn <[email protected]>
@skirpichev
Copy link
Member Author

I think, all merged.

@mkaraev, congratulations!

@mkaraev
Copy link
Contributor

mkaraev commented May 12, 2025

@skirpichev @sobolevn thank you for your help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 bugs and security fixes docs Documentation in the Doc dir easy
Projects
Status: Todo
Development

No branches or pull requests

3 participants