More comments, questions, and suggestions on the latest patch:
1. In _binary_float_to_ratio, the indentation needs adjusting. Also 'top = 0L' could be replaced with 'top =
0', unless you need this code to work with Python 2.3 and earlier, in which case top needs to be a long so
that << behaves correctly. Otherwise, this looks good.
2. Rational() should work, and it should be possible to initialize from a string. I'd suggest that
Rational(' 1729 '), Rational('-3/4') and (' +7/18 \n') should be acceptable: i.e. leading and trailing
whitespace, and an optional - or + sign should be permitted. But space between the optional sign and the
numerator, or on either side of the / sign, should be disallowed. Not sure whether the numerator and
denominator should be allowed to have leading zeros or not---probably yes, for consistency with int().
3. I don't think representing +/-inf and nan as Rationals (1/0, -1/0, 0/0) is a good idea---special values
should be kept out of the Rational type, else it won't be an implementation of the Rationals any more---it'll
be something else.
4. hash(n) doesn't agree with hash(Rational(n)) for large integers (I think you already mentioned this
above).
5. Equality still doesn't work for complex numbers:
>>> from rational import *
>>> Rational(10**23) == complex(10**23) # expect False here
True
>>> Rational(10**23) == float(10**23)
False
>>> float(10**23) == complex(10**23)
True
6. Why the parentheses around the str() of a Rational?
7. How about having hash of a Rational (in the case that it's not equal to an integer or a float) be
given by hash((self.numerator, self.denominator))? That is, let the tuple hash take care of avoiding lots of
hash collisions. |