classification
Title: Add a factorial function
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: ajaksu2, alanmcintyre, avalind, dtorp, georg.brandl, ilan, jafo, maix, mark.dickinson, phr, rhettinger, terry.reedy, uzi
Priority: normal Keywords:

Created on 2008-02-18 10:09 by dtorp, last changed 2010-05-12 19:57 by mark.dickinson. This issue is now closed.

Messages (35)
msg62520 - (view) Author: David Albert Torpey (dtorp) Date: 2008-02-18 10:09
Add a factorial method.  Everybody understands what it means before 
they are out of high school and it comes up all the time in statistics 
and combinatorics.  Ruby has a factorial method and heck even basic 
calculators have a factorial key.

print n.factorial()

Maybe raise ValueError if n is negative.
msg62534 - (view) Author: Alan McIntyre (alanmcintyre) (Python committer) Date: 2008-02-18 15:43
It seems like most of the methods on integers are for two-argument
operations (add, mul, div, etc.), while a lot of single-argument
operations are in the math module.  If this gets added would it fit
better as a function in the math module?

I have to say a factorial function is something I've found myself
writing several times, so I certainly wouldn't mind having one included.
Yeah, it's a one-liner, but so is hypot, and that's already in the
stdlib.  And most calculators don't have a hypot button. ;)
msg62535 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-18 15:46
I don't think it would be appropriate to add this as a method of int; 
it seems like unnecessary clutter on a core type.

Perhaps a math.factorial function could be considered?  Historically,
the math module has just been a way to wrap the (mostly floating-point)
libm functions, but I think that may be changing:  there's at least some
interest in putting gcd into the math module, for example.

David, any chance you could come up with a patch implementing
math.factorial?
msg62536 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-18 15:48
> Yeah, it's a one-liner, but so is hypot

Except that hypot is not a one-liner, if you want to get edge cases
right.  (Consider hypot(1e200, 1e200), or hypot(1e-200, 1e-200).)
msg62539 - (view) Author: Alan McIntyre (alanmcintyre) (Python committer) Date: 2008-02-18 17:50
>Except that hypot is not a one-liner, if you want to get edge cases
right. 

Ah, true; thanks for pointing that out.

Should there be some upper limit on the argument math.factorial would
take?  At the moment I can't think of any reason for picking a given
limit, except perhaps execution time.  (10**4)! can be done in about 1
second on my old & slow laptop; are there realistic use cases for
numbers much bigger than that?
msg62541 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-18 18:12
> Should there be some upper limit on the argument math.factorial would
take?

I'd say not.  Any such limit would be artificial, and an arbitrary
choice.  Just let the natural time and space requirements be the
limiting factor.
msg62549 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-18 22:40
Since the domain and range are ints/longs, this makes more sense as a 
method on <type 'int'> than as a math module function.  The other math 
module functions mostly have real valued inputs and mostly have 
counterparts in cmath.  

IIRC, Tim wanted the math module to maintain that theme and not become 
a home to every function that seems a bit "mathematical". I share the 
sentiment -- it would be nice for the math module to retain its unified 
theme during its growth spurt.
msg62551 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-18 22:55
> Since the domain and range are ints/longs, this makes more sense as a 
> method on <type 'int'> than as a math module function.

Fair enough.  Raymond, do you have any thoughts on where a gcd 
implementation might most usefully live?  Should it stay in 
fractions.py, or is there a case for moving it somewhere more central?
msg62552 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-18 23:04
gcd() is probably fine where it is.  People learn about GCD and LCM 
where they first learn fractions.  So, there is a strong association.
msg62553 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-18 23:05
The other issue here is that I see factorial as being the thin end of 
the 
wedge.  If factorial were implemented, it would probably only be on the 
order of minutes before people wanted to know where the binomial() 
function was.

So it seems to me that either there should be a good single place for 
all 
integer-related math, (gcd, xgcd, binomial, factorial, ...) or we should 
leave this for third-party modules for the moment.
msg62555 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-19 02:39
I wouldn't worry about that so much -- our job is to provide good 
primitives.
msg62557 - (view) Author: David Albert Torpey (dtorp) Date: 2008-02-19 08:28
Mr. Dickinson thank you for doing this.  I do not know how to help with 
a patch.  If it helps, here is the code I use in python:

def factorial(n, _known=[1]):
    assert isinstance(n, int), "Need an integer. This isn't a gamma"
    assert n >= 0, "Sorry, can't factorilize a negative"
    assert n < 1000, "No way! That's too large"
    try:
        return _known[n]
    except IndexError:
        pass
    for i in range(len(_known), n+1):
        _known.append(_known[-1] * i)
    return _known[n]

When the assertions are turned-off, this runs pretty fast.
msg62674 - (view) Author: paul rubin (phr) Date: 2008-02-22 03:53
I like the idea of having some integer math functions like this in the
stdlib; whether in the math module or elsewhere doesn't matter much.  In
particular I remember having to implement xgcd on several separate
occasions for unrelated applications, each time requiring about the same
head scratching as before, and wishing it was in the stdlib so that
could be avoided.  This type of function is complicated enough to not
rattle immediately off the fingertips, but not complicated enough to be
worth looking up in a reference book.

http://bugs.python.org/issue457066 has some discussion of xgcd and
modular inverses.
msg62691 - (view) Author: Daniel Diniz (ajaksu2) Date: 2008-02-22 15:47
Would it be implemented in C? How about using Luschny's Prime Swing
(http://www.luschny.de/math/factorial/FastFactorialFunctions.htm and
http://www.luschny.de/math/factorial/Benchmark.html )?
msg62707 - (view) Author: Anders Valind (avalind) Date: 2008-02-22 23:20
IMHO, The best place to put functions such as xgcd, factorial, etc,
would be a new imath module, an integer equivalent of cmath.

Not only would it keep the standard math module clean, it would also
make clear that these functions are for integers only.
msg62729 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-23 13:58
The trouble is that this comes at a time when people are trying to trim 
the standard library down rather than enlarge it.

Perhaps the solution is a high-quality third party 'imath' module?  
If/when it gets to a stage where lots of people are using it, it could 
be considered for inclusion in the std. lib.
msg62761 - (view) Author: Anders Valind (avalind) Date: 2008-02-23 16:06
Yeah, I like the idea of a third party module and letting the popularity 
and quality decide when/if it will be included.

This would also make it easier to see what kind of functionality people 
would want.
msg63805 - (view) Author: Ilan Schnell (ilan) * Date: 2008-03-17 23:18
The factorial function is most likely to be used in context with other
combinatorial functions (like binomial coefficients) for these functions
an external module seems most appropriate.  Most likely people would
use a factorial function only for some small toy calculation, for those
cases one can always roll a factorial function oneself.  Python should
therefore not be cluttered with this function.  I discussed this with
several developers at PyCon08, and have therefore decided to close the
discussion for now.
msg63816 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-18 00:27
FWIW, I don't agree with the reasoning on the rejection.  Hundreds of
calculator layouts and school textbooks suggest that you can have a
useful factorial function without having to also add binomials and
whatnot.  

The OP requested a simple, widely understood integer method with no
arguments.  That seems very reasonable to me.  A function or method is
not clutter if it has widespread uses and a near zero learning curve.

This is a re-invented function and it would be ashamed to not offer it
because it is a pita every time you need it and it's not already there.
 My guess is that half of long-term Python programmers have written
their own variant at some point but only a small percentage of those
went on to write a binomial coeffient function.

Eventhough this is re-invented often, it is not often re-invented well
(i.e. good error messages for non-integer or negative inputs, a fast
implementation with pre-computed values for small inputs, and being
attached to a namespace where you can find it when needed).  

To compare, I checked the somewhat clean SmallTalk Integer API and found
it had factorial, gcd, and lcm, but not  the other functions mentioned
in the thread.  See: 
http://www.csci.csusb.edu/dick/samples/smalltalk.methods.html#Integer%20methods


Re-opening for further discussion.  If someone still feels that it is a
bad idea, then go ahead and re-close; otherwise, I think we ought to
accept this guy's request.
msg63818 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2008-03-18 00:31
Raymond: Can you come into the core sprint and discuss it with the table
on the right just as you come in the door of the sprint room?  Lian said
that was who he discussed it with and they came to the conclusion to
reject it.
msg63822 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-18 00:36
Wish I could be at the sprint.  I'm back in Los Angeles.  My little post
will have to suffice.  

I thought it was a reasonable request and would hate to see it killed
because other people piled on other requests that were not reasonable. 
I don't buy the reasoning that you have to reject factorial just because
someone else might someday want binomial or a gamma.
msg63823 - (view) Author: paul rubin (phr) Date: 2008-03-18 00:37
Rather than factorial how about a product function, so factorial(n) =
product(xrange(1,n+1)).  I do like the idea of an imath module whether
or not it has factorial, and the topic of this rfe has drifted somewhat
towards the desirability of such a module, which is a reasonable
question for discussion.  I'm more or less indifferent to whether imath
has factorial in it or not.  I do think it would be useful for it to
have functions for things like binomial coefficients that were
implemented a little more cleverly than with large factorials, but in
this case factorial should be there too.
msg63828 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-18 00:47
Problems with product():  It is dreadfully inefficient compared to a
good implementation of factorial.  It has the same name a new itertool
with much different functionality.  The hyper-generalization takes us
further away from the OP's dirt simple request for a piece of
functionality that is widely understood, frequently reimplemented, and
has a near zero learning curve.  If his request is accepted, it does not
preclude you from making some other module with tons of functions of
interest to encryption people, number theorists, and statisticians.
msg63961 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-03-18 18:38
I'm not opposed to adding factorial somewhere, and it doesn't seem
as though anyone else is actively opposed to factorial either.  The
problem is working out where best to put it.  To my inexperienced
eyes, it feels wrong to add it as an int/long method, though I'm
having trouble figuring out exactly why it feels wrong.  It doesn't
fit well with the stuff in the math module either, as Raymond
has pointed out.

There are other integer -> integer functions that I'd consider just
as fundamental and useful as factorial, for a programming language
that has arbitrary-precision integers.  Examples are the integer
square root (i.e. int(floor(sqrt(n)))), or the number of bits in 
an integer (int(floor(log(n, 2))).  I've needed both
of these much more often than factorial.  If the powers that be
accepted a request to add such functions, would they also naturally
become integer methods?

And supposing that gcd were added some day, shouldn't it be in the
same place as factorial?

As to implementation, I'd probably avoid PrimeSwing on the basis
that the added complexity, and cost for future maintainers, just
isn't worth it.  The usual recursive algorithm
(writing n! as (n*(n-2)*(n-4)*...) * ((n-1)*(n-3)*...), and then
applying similar breakdowns to the subproducts) is probably good
enough, together with some caching of small results.

I can volunteer to try to implement this sometime before the 2.6/3.0
betas.
msg63969 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-18 19:18
I prefer factorial as a method (like Ruby and Smalltalk).  Given the
usual notation (n!) or pronounciation (n factorial), it is natural to
write this as:  n.factorial().

Compared to  numbits() and isqrt(), a factorial() method is more basic
in that it is self explanatory and everyone knows what it means by the
time they are in middle school.

FWIW, if separate RFEs were opened for numbits() and isqrt(), I would
support their being int methods too.  Their implementations are helped
by access to the underlying representation, and a case could be made
that these no argument calls are just properties of the number (just
like the sign bit).
msg64913 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2008-04-04 04:59
The fact that other languages have factorial does not in itself impress
me.  What is the actual use case other than illustrating computational
induction (whether by iteration or recursion) and for calculating
binomial coefficients?

I don't really like the idea of making factorial a method for the same
reason that exp, sin, ....... are not methods of float: there is no end.
 People should be able to learn basic classes without specialized
functions attached.
msg67571 - (view) Author: Joshua Uziel (uzi) Date: 2008-05-31 10:33
It's a simplified version, but why not something like this:

import operator
def factorial(num):
    return reduce(operator.mul, range(1, num+1))

A product() function could also be done similarly.
msg67572 - (view) Author: Joshua Uziel (uzi) Date: 2008-05-31 10:40
Or slightly better:

from operator import mul
def factorial(num):
    return reduce(mul, range(2, num+1), 1)
msg67583 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-05-31 15:34
Contrary to what I said above, I'm not going to find time for this
before the beta.  Anyone else want to have a go at producing a patch?

A simple implementation would be fine---the algorithm could be tweaked
for speed later.
msg67588 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-05-31 20:22
I've got in from here.
msg67859 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-06-09 06:55
Added math.factorial() in r64050.
msg68355 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-06-18 00:02
It looks like there's a refcounting bug in the code: if the call to 
PyNumber_Multiply fails then iobj gets DECREF'd twice.  This means that a 
keyboard interrupt of factorial() can exit the interpreter:

Python 2.6a3+ (trunk:64341M, Jun 17 2008, 13:19:01) 
[GCC 4.0.1 (Apple Inc. build 5465)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from math import factorial
[36374 refs]
>>> factorial(10**9)
^CFatal Python error: 
/Users/dickinsm/python_source/trunk/Modules/mathmodule.c:562 object at 
0x81f63c has negative ref count -1
Abort trap
msg69831 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-07-16 20:53
That was fixed by Raymond in 64365.
msg73278 - (view) Author: (maix) Date: 2008-09-15 17:48
I think the unit test is wrong, or at least not as is intended:

You put some numbers in values and shuffle them. But you don't use them
but just range(10) for testing.
msg105605 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-05-12 19:57
maix:  good point.  Fixed in revisions r81126 through r81129.
History
Date User Action Args
2010-05-12 19:57:34mark.dickinsonsetmessages: + msg105605
2008-09-15 17:48:38maixsetnosy: + maix
messages: + msg73278
2008-07-16 20:53:54georg.brandlsetnosy: + georg.brandl
messages: + msg69831
2008-06-18 00:02:03mark.dickinsonsetmessages: + msg68355
2008-06-09 06:55:31rhettingersetstatus: open -> closed
resolution: accepted
messages: + msg67859
2008-05-31 20:22:04rhettingersetassignee: mark.dickinson -> rhettinger
messages: + msg67588
2008-05-31 15:37:31mark.dickinsonsetassignee: mark.dickinson
2008-05-31 15:34:50mark.dickinsonsetmessages: + msg67583
2008-05-31 10:40:51uzisetmessages: + msg67572
2008-05-31 10:33:33uzisetnosy: + uzi
messages: + msg67571
2008-04-04 04:59:58terry.reedysetnosy: + terry.reedy
messages: + msg64913
2008-03-18 19:18:37rhettingersetmessages: + msg63969
2008-03-18 18:38:00mark.dickinsonsetmessages: + msg63961
2008-03-18 00:47:24rhettingersetmessages: + msg63828
2008-03-18 00:37:53phrsetmessages: + msg63823
2008-03-18 00:36:09rhettingersetmessages: + msg63822
2008-03-18 00:31:24jafosetnosy: + jafo
messages: + msg63818
2008-03-18 00:27:21rhettingersetstatus: closed -> open
resolution: rejected -> (no value)
messages: + msg63816
2008-03-17 23:18:36ilansetstatus: open -> closed
title: Factorial -> Add a factorial function
nosy: + ilan
messages: + msg63805
priority: normal
components: + Library (Lib), - Interpreter Core
resolution: rejected
2008-02-23 16:06:39avalindsetmessages: + msg62761
2008-02-23 13:58:27mark.dickinsonsetmessages: + msg62729
2008-02-22 23:20:58avalindsetnosy: + avalind
messages: + msg62707
2008-02-22 15:47:05ajaksu2setnosy: + ajaksu2
messages: + msg62691
2008-02-22 03:53:03phrsetnosy: + phr
messages: + msg62674
2008-02-19 08:28:10dtorpsetmessages: + msg62557
2008-02-19 02:39:07rhettingersetmessages: + msg62555
2008-02-18 23:05:52mark.dickinsonsetmessages: + msg62553
2008-02-18 23:04:30rhettingersetmessages: + msg62552
2008-02-18 22:55:26mark.dickinsonsetmessages: + msg62551
2008-02-18 22:40:58rhettingersetnosy: + rhettinger
messages: + msg62549
2008-02-18 18:12:19mark.dickinsonsetmessages: + msg62541
2008-02-18 17:50:51alanmcintyresetmessages: + msg62539
2008-02-18 15:48:30mark.dickinsonsetmessages: + msg62536
2008-02-18 15:46:09mark.dickinsonsetnosy: + mark.dickinson
messages: + msg62535
2008-02-18 15:43:47alanmcintyresetnosy: + alanmcintyre
messages: + msg62534
2008-02-18 10:09:27dtorpcreate