math.factorial doc should mention integer return type

[JohnYeung]

BPO	25735
Nosy	@rhettinger, @terryjreedy, @facundobatista, @mdickinson, @skrah, @csabella, @miss-islington
PRs	bpo-25735: math.factorial doc should mention integer return type #6420 [3.7] bpo-25735: math.factorial doc should mention integer return type (GH-6420) #13702
Files	bug.patch: edited math.rst

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-31.16:46:33.420>
created_at = <Date 2015-11-25.19:44:31.461>
labels = ['3.7', '3.8', 'docs']
title = 'math.factorial doc should mention integer return type'
updated_at = <Date 2019-05-31.16:58:30.519>
user = 'https://bugs.python.org/JohnYeung'

bugs.python.org fields:

activity = <Date 2019-05-31.16:58:30.519>
actor = 'miss-islington'
assignee = 'docs@python'
closed = True
closed_date = <Date 2019-05-31.16:46:33.420>
closer = 'cheryl.sabella'
components = ['Documentation']
creation = <Date 2015-11-25.19:44:31.461>
creator = 'John.Yeung'
dependencies = []
files = ['41206']
hgrepos = []
issue_num = 25735
keywords = ['patch']
message_count = 11.0
messages = ['255382', '255479', '255542', '255693', '313909', '313910', '313911', '313913', '344088', '344092', '344093']
nosy_count = 10.0
nosy_names = ['rhettinger', 'terry.reedy', 'facundobatista', 'mark.dickinson', 'skrah', 'docs@python', 'John.Yeung', 'mine0901', 'cheryl.sabella', 'miss-islington']
pr_nums = ['6420', '13702']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25735'
versions = ['Python 3.7', 'Python 3.8']

[JohnYeung]

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).

[terryjreedy]

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.

[mdickinson]

[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.)

[mine0901]

States that math.factorial(x) returns x factorial as an integer.

[csabella]

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!

[mdickinson]

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?

[csabella]

Sounds good. Thanks, Mark.

@mine0901, would you like to open a Github pull request for your patch? Thanks!

[mdickinson]

I opened bpo-33083, and copied the nosy list from this issue across.

[csabella]

New changeset 4612671 by Cheryl Sabella (Akshay Sharma) in branch 'master':
bpo-25735: math.factorial doc should mention integer return type (GH-6420)
4612671

[csabella]

@John.Yeung, thank you for the report. @mine0901, thanks for the initial patch and @akshaysharma, thank you for the PR.

[miss-islington]

New changeset fc3b843 by Miss Islington (bot) in branch '3.7':
bpo-25735: math.factorial doc should mention integer return type (GH-6420)
fc3b843