Some random comments: take with a large pinch of salt
(1) In __init__ I think you want something like:
self._numerator = int(numerator)//g
instead of
self._numerator = int(numerator / g)
and similarly for the denominator. At the moment I get, for example:
>>> Rational(10**23)
rational.Rational(99999999999999991611392,1)
(2) What's the reason for repr of a Rational being "rational.Rational(...)", while repr
of a Decimal is just "Decimal(...)"? I'm not suggesting that either is wrong; just
wondering whether there's some sort of guiding principle that would suggest one or the
other.
(3) My first thought on looking at the _gcd function was: "Shouldn't there be an abs()
somewhere in there"; since the gcd of two (possibly negative) integers is often
(usually?) defined as the largest *nonnegative* common divisor. But on closer
examination it's clear that the result of _gcd(a, b) has the same sign as that of b
(unless b == 0). So _gcd very cleverly chooses exactly the right sign so that the
denominator after rescaling is positive. I'm not sure whether this is a happy accident
or clever design, but either way it probably deserves a comment.
(4) the __ceil__ operation could be spelt more neatly as
return -(-a.numerator // a.denominator)
...but perhaps that just amounts to obfuscation :)
(5) It looks as though two-argument round just truncates when the second argument is
negative. Presmably this isn't what's wanted here?
>>> round(Rational(26), -1) # expecting Rational(30, 1)
rational.Rational(20,1)
(6) The behaviour of the equality test is questionable when floats are involved. For
example:
>>> 10**23 == float(10**23) # expect False
False
>>> Rational(10**23) == float(10**23) # I'd also expect False here
True
Ideally, if x is a Rational and y is a float then I'd suggest that x == y should return
True only when the numeric values really are equal. Coding this could be quite tricky,
though. One option would be to convert the float to an (exactly equal) Rational first--
-there's code to do this in the version of Rational.py in the sandbox.
(7) For purely selfish reasons, I for one will be very happy if/when this makes it into
2.6/3.0: I use Python a lot for teaching (geometry, number theory, linear algebra,
...); it's natural to work with rational numbers in this context; and it'll be much
easier to tell an interested student to just download Python than to tell them they also
need gmp and gmpy, or some other third party package, just to try out the code examples
I've given them. |