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 wolma
Recipients mark.dickinson, wolma
Date 2017-11-08.10:13:02
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1510135982.93.0.213398074469.issue31978@psf.upfronthosting.co.za>
In-reply-to
Content
Hi,

because of floating point inaccuracies it is suboptimal to use round(int1/int2) for rounding of a fraction.
fractions.Fraction, OTOH, offers exact rounding through its implementation of __round__, but using it requires users to create a fractions.Fraction instance from two ints first.
The algorithm used by Fraction.__round__ is, essentially, the same as the one used in the pure-Python version of the datetime._divide_and_round module (which, in turn, is taken from a comment by Mark Dickinson in Objects/longobject.c).

My suggestion is to promote this algorithm to an exposed function in the fractions module (actually, it may make sense to have it in the math module instead - compare the case of the gcd function, which started out in fractions, but is now in transition to math) so that it can be used by Fraction.__round__, but also any other code.

Attached is a patch demonstrating the idea. In addition, to the above benefit, it turns out to actually speed up Fraction.__round__ substantially, when ndigits is not None because it then avoids the generation of temporary Fraction instances, and, in my hands at least, it even makes the general (ndigits=None) case slightly faster (apparently the copied datetime._divide_and_round code is more efficient than the original in Fraction.__round__).

There is one slight additional tweak in the patch: in the case of ndigits < 0, it returns an int, not a Fraction (see test_fractions modification to make it pass).
I think this is actually a mistake in the current Fraction.__round__, which already promotes the result to int in the general case. This change speeds up round to the next ndigits power of ten by ~ a factor of 5 in my hands because no new Fraction needs to be instantiated anymore.

A full PR could include having pure-Python datetime import the function from fractions instead of rolling its own, but I'd first like to hear whether you think this should go into math instead.
History
Date User Action Args
2017-11-08 10:13:02wolmasetrecipients: + wolma, mark.dickinson
2017-11-08 10:13:02wolmasetmessageid: <1510135982.93.0.213398074469.issue31978@psf.upfronthosting.co.za>
2017-11-08 10:13:02wolmalinkissue31978 messages
2017-11-08 10:13:02wolmacreate