From da915c0b7e93bf8bd16cb5a9151b3e37ac474c71 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 19 Mar 2018 00:41:21 +0000 Subject: [PATCH 1/6] Use PyNumber_Index in the non-float branch of math_factorial and add deprecation warning for floats --- Lib/test/test_math.py | 32 ++++++++++++++++++++------------ Modules/mathmodule.c | 18 +++++++++++++++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py index fff82fe7fadb97b..8e7eaf6c5ca2829 100644 --- a/Lib/test/test_math.py +++ b/Lib/test/test_math.py @@ -5,6 +5,7 @@ from test import support import unittest import itertools +import decimal import math import os import platform @@ -497,18 +498,24 @@ 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 @@ -516,7 +523,8 @@ 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 index 8015a95d2aa7462..496233bfe279f18 100644 --- 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; From 5ebef8b0d497f7cf496063422e86189f5c026ff2 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 19 Mar 2018 00:59:28 +0000 Subject: [PATCH 2/6] Add News entry --- .../Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst b/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst new file mode 100644 index 000000000000000..b3ad84aaf20957c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst @@ -0,0 +1,3 @@ +math.factorial no longer accept arguments that are not int-like. A +DeprecationWarning is emmited if the argument is a float. Patch by Pablo +Galindo. From 0494f10da3f63eb8d7764e4b003b0d7b1bba0df8 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 30 Jul 2018 13:23:41 +0100 Subject: [PATCH 3/6] Remove deprecation warning for floats --- Lib/test/test_math.py | 27 ++++++++++++--------------- Modules/mathmodule.c | 8 +------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_math.py b/Lib/test/test_math.py index 8e7eaf6c5ca2829..608789f7202f035 100644 --- a/Lib/test/test_math.py +++ b/Lib/test/test_math.py @@ -498,20 +498,18 @@ def testFabs(self): def testFactorial(self): self.assertEqual(math.factorial(0), 1) - 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.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) - 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) + 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)) @@ -523,8 +521,7 @@ 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) - with self.assertWarns(DeprecationWarning): - self.assertRaises(OverflowError, math.factorial, 1e100) + self.assertRaises(OverflowError, math.factorial, 1e100) def testFloor(self): self.assertRaises(TypeError, math.floor) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 496233bfe279f18..510c569ce0e8420 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -1671,16 +1671,10 @@ 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){ + if(pyint_form == NULL){ return NULL; } x = PyLong_AsLongAndOverflow(pyint_form, &overflow); From 7a1ed88b8baf27b33a03f9b8bd09a2c0fd092cb8 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 30 Jul 2018 13:40:33 +0100 Subject: [PATCH 4/6] Correct NEWS entry --- .../2018-03-19-00-59-20.bpo-33083.Htztjl.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst b/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst index b3ad84aaf20957c..c2140b195f65147 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst @@ -1,3 +1,3 @@ -math.factorial no longer accept arguments that are not int-like. A -DeprecationWarning is emmited if the argument is a float. Patch by Pablo +math.factorial no longer accepts arguments that are not int-like. A +DeprecationWarning is emitted if the argument is a float. Patch by Pablo Galindo. From 33060fe858c6a99c9e6b2cc6a67a167d7a6bd625 Mon Sep 17 00:00:00 2001 From: Tal Einat Date: Mon, 30 Jul 2018 21:39:21 +0300 Subject: [PATCH 5/6] add a few missing spaces --- Modules/mathmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 510c569ce0e8420..287184cceb77384 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -1672,9 +1672,9 @@ math_factorial(PyObject *module, PyObject *arg) x = PyLong_AsLongAndOverflow(lx, &overflow); Py_DECREF(lx); } - else{ + else { pyint_form = PyNumber_Index(arg); - if(pyint_form == NULL){ + if (pyint_form == NULL) { return NULL; } x = PyLong_AsLongAndOverflow(pyint_form, &overflow); From 1c5bd8689bbcbb231c0c4a7e9e505e45d8ec6442 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Mon, 30 Jul 2018 20:13:32 +0100 Subject: [PATCH 6/6] Correct NEWS entry --- .../2018-03-19-00-59-20.bpo-33083.Htztjl.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst b/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst index c2140b195f65147..81df83989e48e39 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2018-03-19-00-59-20.bpo-33083.Htztjl.rst @@ -1,3 +1,2 @@ -math.factorial no longer accepts arguments that are not int-like. A -DeprecationWarning is emitted if the argument is a float. Patch by Pablo -Galindo. +``math.factorial`` no longer accepts arguments that are not int-like. +Patch by Pablo Galindo.