Use PyNumber_Index in the non-float branch of math_factorial and add deprecation warning for floats

pablogsal · pablogsal · commit 9ec8b612a134 · 2018-03-19T00:56:33.000Z
diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py
@@ -4,6 +4,7 @@
 from test.support import run_unittest, verbose, requires_IEEE_754
 from test import support
 import unittest
+import decimal
 import math
 import os
 import platform
@@ -494,26 +495,33 @@ def testFabs(self):
 
     def testFactorial(self):
         self.assertEqual(math.factorial(0), 1)
-        self.assertEqual(math.factorial(0.0), 1)
-        total = 1
-        for i in range(1, 1000):
-            total *= i
-            self.assertEqual(math.factorial(i), total)
-            self.assertEqual(math.factorial(float(i)), total)
-            self.assertEqual(math.factorial(i), py_factorial(i))
+        with self.assertWarns(DeprecationWarning):
+            self.assertEqual(math.factorial(0.0), 1)
+            total = 1
+            for i in range(1, 1000):
+                total *= i
+                self.assertEqual(math.factorial(i), total)
+                self.assertEqual(math.factorial(float(i)), total)
+                self.assertEqual(math.factorial(i), py_factorial(i))
         self.assertRaises(ValueError, math.factorial, -1)
-        self.assertRaises(ValueError, math.factorial, -1.0)
-        self.assertRaises(ValueError, math.factorial, -10**100)
-        self.assertRaises(ValueError, math.factorial, -1e100)
-        self.assertRaises(ValueError, math.factorial, math.pi)
+        with self.assertWarns(DeprecationWarning):
+            self.assertRaises(ValueError, math.factorial, -1.0)
+            self.assertRaises(ValueError, math.factorial, -10**100)
+            self.assertRaises(ValueError, math.factorial, -1e100)
+            self.assertRaises(ValueError, math.factorial, math.pi)
+
+    def testFactorialNonIntegers(self):
+        self.assertRaises(TypeError, math.factorial, decimal.Decimal(5.2))
+        self.assertRaises(TypeError, math.factorial, "5")
 
     # Other implementations may place different upper bounds.
     @support.cpython_only
     def testFactorialHugeInputs(self):
         # Currently raises ValueError for inputs that are too large
         # to fit into a C long.
         self.assertRaises(OverflowError, math.factorial, 10**100)
-        self.assertRaises(OverflowError, math.factorial, 1e100)
+        with self.assertWarns(DeprecationWarning):
+            self.assertRaises(OverflowError, math.factorial, 1e100)
 
     def testFloor(self):
         self.assertRaises(TypeError, math.floor)
diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c
@@ -1656,7 +1656,7 @@ math_factorial(PyObject *module, PyObject *arg)
 {
     long x;
     int overflow;
-    PyObject *result, *odd_part, *two_valuation;
+    PyObject *result, *odd_part, *two_valuation, *pyint_form;
 
     if (PyFloat_Check(arg)) {
         PyObject *lx;
@@ -1671,9 +1671,21 @@ math_factorial(PyObject *module, PyObject *arg)
             return NULL;
         x = PyLong_AsLongAndOverflow(lx, &overflow);
         Py_DECREF(lx);
+        if (PyErr_WarnEx(PyExc_DeprecationWarning,
+                "Passing float instances to math.factorial will be "
+                "removed in future versions.", 1) < 0) {
+            return NULL;
+        }
+
+    }
+    else{
+        pyint_form = PyNumber_Index(arg);
+        if( pyint_form == NULL){
+            return NULL;
+        }
+        x = PyLong_AsLongAndOverflow(pyint_form, &overflow);
+        Py_DECREF(pyint_form);
     }
-    else
-        x = PyLong_AsLongAndOverflow(arg, &overflow);
 
     if (x == -1 && PyErr_Occurred()) {
         return NULL;
