| msg255382 - (view) |
Author: John Yeung (John.Yeung) |
Date: 2015-11-25 19:44 |
| The math module docs state Except when explicitly noted otherwise, all return values are floats. But math.factorial isn't what I would call explicit about returning int: math.factorial(x) Return x factorial. Raises ValueError if x is not integral or is negative. At minimum, shouldn't the first sentence be "Return x factorial as an int."? I haven't tested on all Python versions, but math.factorial on 2.7 and 3.2 definitely return int (or long in Python 2 when necessary). |
|
|
| msg255479 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2015-11-27 19:53 |
| I agree. In testing, I discovered this bug >>> factorial(decimal.Decimal(5.2)) 120 I don't know if this is a glitch in factorial or Decimal. I also noticed >>> fac(fractions.Fraction(4, 1)) Traceback (most recent call last): File "<pyshell#10>", line 1, in fac(fractions.Fraction(4, 1)) TypeError: an integer is required (got type Fraction) Perhaps this is due to no __int__ method. |
|
|
| msg255542 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2015-11-28 14:29 |
| [Terry] >>> factorial(decimal.Decimal(5.2)) 120 Yep, that's definitely wrong. If we want to behave the same way as for float, we should accept only integral Decimal values. (Though I'm not much of a fan of the float behaviour: I would have preferred math.factorial not to accept floats at all.) |
|
|
| msg255693 - (view) |
Author: Sonali Gupta (mine0901) * |
Date: 2015-12-02 06:49 |
| States that math.factorial(x) returns x factorial as an integer. |
|
|
| msg313909 - (view) |
Author: Cheryl Sabella (cheryl.sabella) * |
Date: 2018-03-15 20:23 |
| Should the documentation patch for this be converted to a PR or does this need to change to a bug issue to address Mark's concerns? Thanks! |
|
|
| msg313910 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2018-03-15 20:27 |
| > Should the documentation patch for this be converted to a PR I think so, yes. How about we open a new issue for the factorial(Decimal(...)) behaviour, and keep this one focused on the original reported issue? |
|
|
| msg313911 - (view) |
Author: Cheryl Sabella (cheryl.sabella) * |
Date: 2018-03-15 20:34 |
| Sounds good. Thanks, Mark. @mine0901, would you like to open a Github pull request for your patch? Thanks! |
|
|
| msg313913 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2018-03-15 20:39 |
| I opened #33083, and copied the nosy list from this issue across. |
|
|
| msg344088 - (view) |
Author: Cheryl Sabella (cheryl.sabella) * |
Date: 2019-05-31 16:41 |
| New changeset 4612671df2742eade8ecf8003a6ce1247973c135 by Cheryl Sabella (Akshay Sharma) in branch 'master': bpo-25735: math.factorial doc should mention integer return type (GH-6420) https://github.com/python/cpython/commit/4612671df2742eade8ecf8003a6ce1247973c135 |
|
|
| msg344092 - (view) |
Author: Cheryl Sabella (cheryl.sabella) * |
Date: 2019-05-31 16:46 |
| @John.Yeung, thank you for the report. @mine0901, thanks for the initial patch and @akshaysharma, thank you for the PR. |
|
|
| msg344093 - (view) |
Author: miss-islington (miss-islington) |
Date: 2019-05-31 16:58 |
| New changeset fc3b8437c86167983cc75e5a9a3ed1dc542a1b79 by Miss Islington (bot) in branch '3.7': bpo-25735: math.factorial doc should mention integer return type (GH-6420) https://github.com/python/cpython/commit/fc3b8437c86167983cc75e5a9a3ed1dc542a1b79 |
|
|