Increase speed of Fractions.limit_denominator

[mscuthbert]

Feature or enhancement

Speed up Fractions.limit_denominator by not creating intermediary Fraction objects

Pitch

Fractions.limit_denominator returns a new Fraction, but along the way for Fractions > max_denominator creates 3 Fraction objects that are discarded: the fraction farther from the bound and two temporary fractions (bound1 - self and bound2 - shelf) which are created just for getting their absolute values.

Similar code speedups have been part of the music21 library since 2015--where we call limit_denominator after every arithmetic operation on note placement--and timing tests give a 7.3x speedup. (see music21 Source )

For fractions with denominators equal to or below max_denominator, instantiate the new Fraction with Fraction(self._numerator, self._denominator) instead of Fraction(self) for a 23% speedup.

PR to follow.

[rhettinger]

In this case, we value algorithmic clarity over a minor performance gain from inlining.

Also, we would lose some of the recent optimizations in the Fraction class.

The limit_denominator code is almost never in a performance critical section of code, so no one would notice the small benefit, but we would incur a loss of understanding about how it works.

Thanks for the suggestion, but we will decline this one.