➜
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2015-11-25 19:44 byJohn.Yeung, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| bug.patch | mine0901,2015-12-02 06:49 | edited math.rst | review | |
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 6420 | merged | akshaysharma,2018-04-08 13:42 | |
| PR 13702 | merged | miss-islington,2019-05-31 16:43 | |
| Messages (11) | |||
|---|---|---|---|
| msg255382 -(view) | Author: John Yeung (John.Yeung) | Date: 2015-11-25 19:44 | |
The math module docs stateExcept 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))120I 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 <module> 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))120Yep, 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 PRI 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 changeset4612671df2742eade8ecf8003a6ce1247973c135 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 changesetfc3b8437c86167983cc75e5a9a3ed1dc542a1b79 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 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:24 | admin | set | github: 69921 |
| 2019-05-31 16:58:30 | miss-islington | set | nosy: +miss-islington messages: +msg344093 |
| 2019-05-31 16:46:33 | cheryl.sabella | set | status: open -> closed resolution: fixed messages: +msg344092 stage: patch review -> resolved |
| 2019-05-31 16:43:16 | miss-islington | set | pull_requests: +pull_request13588 |
| 2019-05-31 16:41:34 | cheryl.sabella | set | messages: +msg344088 |
| 2019-05-31 09:53:36 | xtreak | link | issue37109 superseder |
| 2018-04-08 13:42:47 | akshaysharma | set | stage: needs patch -> patch review pull_requests: +pull_request6123 |
| 2018-03-15 20:39:08 | mark.dickinson | set | messages: +msg313913 |
| 2018-03-15 20:34:02 | cheryl.sabella | set | messages: +msg313911 |
| 2018-03-15 20:27:04 | mark.dickinson | set | messages: +msg313910 |
| 2018-03-15 20:23:14 | cheryl.sabella | set | versions: + Python 3.7, Python 3.8, - Python 3.5 nosy: +cheryl.sabella messages: +msg313909 stage: needs patch |
| 2015-12-02 06:49:10 | mine0901 | set | files: +bug.patch nosy: +mine0901 messages: +msg255693 keywords: +patch |
| 2015-11-28 14:29:08 | mark.dickinson | set | messages: +msg255542 |
| 2015-11-27 19:53:29 | terry.reedy | set | nosy: +rhettinger,skrah,terry.reedy,facundobatista,mark.dickinson messages: +msg255479 |
| 2015-11-25 19:44:31 | John.Yeung | create | |