This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients John.Yeung, facundobatista, mark.dickinson, rhettinger, serhiy.storchaka, taleinat, terry.reedy, tim.peters, vstinner
Date 2018-07-31.09:59:08
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <>
I looked at Pablo's PR 6149 and I'm surprised by the size of the code just to convert a Python object to a C long!
    if (PyFloat_Check(arg)) {
        PyObject *lx;
        double dx = PyFloat_AS_DOUBLE((PyFloatObject *)arg);
        if (!(Py_IS_FINITE(dx) && dx == floor(dx))) {
                            "factorial() only accepts integral values");
            return NULL;
        lx = PyLong_FromDouble(dx);
        if (lx == NULL)
            return NULL;
        x = PyLong_AsLongAndOverflow(lx, &overflow);
    else {
        pyint_form = PyNumber_Index(arg);
        if (pyint_form == NULL) {
            return NULL;
        x = PyLong_AsLongAndOverflow(pyint_form, &overflow);

    if (x == -1 && PyErr_Occurred()) {
        return NULL;
    else if (overflow == 1) {
                     "factorial() argument should not exceed %ld",
        return NULL;
    else if (overflow == -1 || x < 0) {
                        "factorial() not defined for negative values");
        return NULL;
Do we really need 37 lines of C code? Is it really important to accept integral float? What's wrong with factorial(int(d)) for example?

PR 6149 LGTM, but since I was not involved in the discussion, I would prefer if someone else who has been involved would approve the change: Tal already approved it, but I saw some complains, I'm not 100% sure that we reached a full agreement.

Note: Pablo is a core developer and so he can merge his PR, I'm only asking to *approve* the change, not to merge it ;-)

Thanks in advance.

Note2: I'm still mentoring Pablo after he became a core, and I require him to ask me before merging any change.
Date User Action Args
2018-07-31 09:59:08vstinnersetrecipients: + vstinner, tim.peters, rhettinger, terry.reedy, facundobatista, mark.dickinson, taleinat, serhiy.storchaka, John.Yeung
2018-07-31 09:59:08vstinnersetmessageid: <>
2018-07-31 09:59:08vstinnerlinkissue33083 messages
2018-07-31 09:59:08vstinnercreate