Issue #1996: float.as_integer_ratio() should return fraction in lowest terms.

diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py
index 59a9dcd3e07df99e4f58ba986f336614af7a4502..2f2634d054f38baa905061fac7ff3d9eb4006fe6 100644 (file)
--- a/Lib/test/test_builtin.py
+++ b/Lib/test/test_builtin.py
@@ -689,6 +689,14 @@ class BuiltinTest(unittest.TestCase):
         self.assertRaises(TypeError, float, Foo4(42))
 
     def test_floatasratio(self):
+        for f, ratio in [
+                (0.875, (7, 8)),
+                (-0.875, (-7, 8)),
+                (0.0, (0, 1)),
+                (11.5, (23, 2)),
+            ]:
+            self.assertEqual(f.as_integer_ratio(), ratio)
+
         R = rational.Rational
         self.assertEqual(R(0, 1),
                          R(*float(0.0).as_integer_ratio()))

diff --git a/Objects/floatobject.c b/Objects/floatobject.c
index 63512787a6b46565d7ae21701220474bf681480c..a3733fcd9cdd1daccc61a095cd0c8b42a47b3f5f 100644 (file)
--- a/Objects/floatobject.c
+++ b/Objects/floatobject.c
@@ -1154,20 +1154,18 @@ float_float(PyObject *v)
 }
 
 static PyObject *
-float_as_integer_ratio(PyObject *v)
+float_as_integer_ratio(PyObject *v, PyObject *unused)
 {
        double self;
        double float_part;
        int exponent;
-       int is_negative;
-       const int chunk_size = 28;
+
        PyObject *prev;
-       PyObject *py_chunk = NULL;
        PyObject *py_exponent = NULL;
        PyObject *numerator = NULL;
        PyObject *denominator = NULL;
        PyObject *result_pair = NULL;
-       PyNumberMethods *long_methods;
+       PyNumberMethods *long_methods = PyLong_Type.tp_as_number;
 
 #define INPLACE_UPDATE(obj, call) \
        prev = obj; \
@@ -1189,85 +1187,22 @@ float_as_integer_ratio(PyObject *v)
        }
 #endif
 
-       if (self == 0) {
-               numerator = PyInt_FromLong(0);
-               if (numerator == NULL) goto error;
-               denominator = PyInt_FromLong(1);
-               if (denominator == NULL) goto error;
-               result_pair = PyTuple_Pack(2, numerator, denominator);
-               /* Hand ownership over to the tuple. If the tuple
-                  wasn't created successfully, we want to delete the
-                  ints anyway. */
-               Py_DECREF(numerator);
-               Py_DECREF(denominator);
-               return result_pair;
-       }
-
-       /* XXX: Could perhaps handle FLT_RADIX!=2 by using ilogb and
-          scalbn, but those may not be in C89. */
        PyFPE_START_PROTECT("as_integer_ratio", goto error);
-       float_part = frexp(self, &exponent);
-       is_negative = 0;
-       if (float_part < 0) {
-               float_part = -float_part;
-               is_negative = 1;
-               /* 0.5 <= float_part < 1.0 */
-       }
+       float_part = frexp(self, &exponent);    /* self == float_part * 2**exponent exactly */
        PyFPE_END_PROTECT(float_part);
-       /* abs(self) == float_part * 2**exponent exactly */
-
-       /* Suck up chunk_size bits at a time; 28 is enough so that we
-          suck up all bits in 2 iterations for all known binary
-          double-precision formats, and small enough to fit in a
-          long. */
-       numerator = PyLong_FromLong(0);
-       if (numerator == NULL) goto error;
-
-       long_methods = PyLong_Type.tp_as_number;
-
-       py_chunk = PyLong_FromLong(chunk_size);
-       if (py_chunk == NULL) goto error;
-
-       while (float_part != 0) {
-               /* invariant: abs(self) ==
-                  (numerator + float_part) * 2**exponent exactly */
-               long digit;
-               PyObject *py_digit;
-
-               PyFPE_START_PROTECT("as_integer_ratio", goto error);
-               /* Pull chunk_size bits out of float_part, into digits. */
-               float_part = ldexp(float_part, chunk_size);
-               digit = (long)float_part;
-               float_part -= digit;
-                /* 0 <= float_part < 1 */
-               exponent -= chunk_size;
-               PyFPE_END_PROTECT(float_part);
-
-               /* Shift digits into numerator. */
-               // numerator <<= chunk_size
-               INPLACE_UPDATE(numerator,
-                              long_methods->nb_lshift(numerator, py_chunk));
-               if (numerator == NULL) goto error;
-
-               // numerator |= digit
-               py_digit = PyLong_FromLong(digit);
-               if (py_digit == NULL) goto error;
-               INPLACE_UPDATE(numerator,
-                              long_methods->nb_or(numerator, py_digit));
-               Py_DECREF(py_digit);
-               if (numerator == NULL) goto error;
+       
+       while (float_part != floor(float_part)) {
+               float_part *= 2.0;
+               exponent--;
        }
+       /* Now, self == float_part * 2**exponent exactly and float_part is integral */
 
-       /* Add in the sign bit. */
-       if (is_negative) {
-               INPLACE_UPDATE(numerator,
-                              long_methods->nb_negative(numerator));
-               if (numerator == NULL) goto error;
-       }
+       numerator = PyLong_FromDouble(float_part);
+       if (numerator == NULL) goto error;
 
        /* now self = numerator * 2**exponent exactly; fold in 2**exponent */
-       denominator = PyLong_FromLong(1);
-       py_exponent = PyLong_FromLong(labs(exponent));
+       denominator = PyInt_FromLong(1);
+       py_exponent = PyInt_FromLong(labs(exponent));
        if (py_exponent == NULL) goto error;
        INPLACE_UPDATE(py_exponent,
                       long_methods->nb_lshift(denominator, py_exponent));
@@ -1289,7 +1224,6 @@ float_as_integer_ratio(PyObject *v)
 #undef INPLACE_UPDATE
 error:
        Py_XDECREF(py_exponent);
-       Py_XDECREF(py_chunk);
        Py_XDECREF(denominator);
        Py_XDECREF(numerator);
        return result_pair;
@@ -1298,17 +1232,16 @@ error:
 PyDoc_STRVAR(float_as_integer_ratio_doc,
 "float.as_integer_ratio() -> (int, int)\n"
 "\n"
-"Returns a pair of integers, not necessarily in lowest terms, whose\n"
-"ratio is exactly equal to the original float. This method raises an\n"
-"OverflowError on infinities and a ValueError on nans. The resulting\n"
-"denominator will be positive.\n"
+"Returns a pair of integers, whose ratio is exactly equal to the original\n"
+"float and with a positive denominator.\n"
+"Raises OverflowError on infinities and a ValueError on nans.\n"
 "\n"
 ">>> (10.0).as_integer_ratio()\n"
-"(167772160L, 16777216L)\n"
+"(10, 1)\n"
 ">>> (0.0).as_integer_ratio()\n"
 "(0, 1)\n"
 ">>> (-.25).as_integer_ratio()\n"
-"(-134217728L, 536870912L)");
+"(-1, 4)");
 
 
 static PyObject *