This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author mark.dickinson
Recipients gvanrossum, jyasskin, mark.dickinson
Date 2008-01-07.01:31:47
SpamBayes Score 0.017567504
Marked as misclassified No
Message-id <1199669510.7.0.840825269343.issue1682@psf.upfronthosting.co.za>
In-reply-to
Content
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.
History
Date User Action Args
2008-01-07 01:31:50mark.dickinsonsetspambayes_score: 0.0175675 -> 0.017567504
recipients: + mark.dickinson, gvanrossum, jyasskin
2008-01-07 01:31:50mark.dickinsonsetspambayes_score: 0.0175675 -> 0.0175675
messageid: <1199669510.7.0.840825269343.issue1682@psf.upfronthosting.co.za>
2008-01-07 01:31:49mark.dickinsonlinkissue1682 messages
2008-01-07 01:31:48mark.dickinsoncreate