classification
Title: math.factorial doc should mention integer return type
Type: Stage: resolved
Components: Documentation Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: John.Yeung, cheryl.sabella, docs@python, facundobatista, mark.dickinson, mine0901, miss-islington, rhettinger, skrah, terry.reedy
Priority: normal Keywords: patch

Created on 2015-11-25 19:44 by John.Yeung, last changed 2019-05-31 16:58 by miss-islington. This issue is now closed.

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 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) * (Python committer) 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 <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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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
History
Date User Action Args
2019-05-31 16:58:30miss-islingtonsetnosy: + miss-islington
messages: + msg344093
2019-05-31 16:46:33cheryl.sabellasetstatus: open -> closed
resolution: fixed
messages: + msg344092

stage: patch review -> resolved
2019-05-31 16:43:16miss-islingtonsetpull_requests: + pull_request13588
2019-05-31 16:41:34cheryl.sabellasetmessages: + msg344088
2019-05-31 09:53:36xtreaklinkissue37109 superseder
2018-04-08 13:42:47akshaysharmasetstage: needs patch -> patch review
pull_requests: + pull_request6123
2018-03-15 20:39:08mark.dickinsonsetmessages: + msg313913
2018-03-15 20:34:02cheryl.sabellasetmessages: + msg313911
2018-03-15 20:27:04mark.dickinsonsetmessages: + msg313910
2018-03-15 20:23:14cheryl.sabellasetversions: + Python 3.7, Python 3.8, - Python 3.5
nosy: + cheryl.sabella

messages: + msg313909

stage: needs patch
2015-12-02 06:49:10mine0901setfiles: + bug.patch

nosy: + mine0901
messages: + msg255693

keywords: + patch
2015-11-28 14:29:08mark.dickinsonsetmessages: + msg255542
2015-11-27 19:53:29terry.reedysetnosy: + rhettinger, skrah, terry.reedy, facundobatista, mark.dickinson
messages: + msg255479
2015-11-25 19:44:31John.Yeungcreate