Changes by Wolfgang Maier wolfgang.ma...@biologie.uni-freiburg.de:
--
nosy: +wolma
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1682
___
___
Mark Dickinson [EMAIL PROTECTED] added the comment:
Well, I can't find anything more to fuss about here. :-)
Reclosing.
--
status: open - closed
___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
Mark Dickinson [EMAIL PROTECTED] added the comment:
I agree that if the type of the 2nd arg isn't int or long it should be
rejected. That should not slow down the common path (two ints).
I'm having second thoughts about this; it doesn't seem worth adding an
extra check that has to be
Guido van Rossum [EMAIL PROTECTED] added the comment:
That's fine.
___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
___
___
Python-bugs-list mailing list
Mark Dickinson [EMAIL PROTECTED] added the comment:
Trailing 'L's removed in r64557.
A couple more minor issues:
(1) fractions.gcd is exported but not documented. I'll add some
documentation for it, unless anyone feels that it shouldn't have been
exported in the first place. If so, please
Mark Dickinson [EMAIL PROTECTED] added the comment:
Hmm. I don't much like this, though:
Fraction('1.23', 1.0)
Fraction(123, 100)
Fraction('1.23', 1+0j)
Fraction(123, 100)
I'd say these should definitely be TypeErrors.
___
Python tracker [EMAIL
Guido van Rossum [EMAIL PROTECTED] added the comment:
I think it's okay to accept Fraction('1.23', 1) -- if only because
rejecting it means changing the default for the 2nd arg to None and that
would slow it down again.
I agree that if the type of the 2nd arg isn't int or long it should be
Mark Dickinson [EMAIL PROTECTED] added the comment:
I agree that if the type of the 2nd arg isn't int or long it should be
rejected. That should not slow down the common path (two ints).
Sounds good. Some care might be needed to avoid invalidating
Fraction(n, d) where n and d are instances
Mark Dickinson [EMAIL PROTECTED] added the comment:
I'm reopening this to propose a last-minute minor change:
Can we remove the trailing 'L' from the numerator and denominator in
the repr of a Fraction instance? E.g.,
x = Fraction('9.876543210')
x
Fraction(987654321L, 1L)
x * (1/x)
Raymond Hettinger [EMAIL PROTECTED] added the comment:
+1 on removing the L. Also, it would be nice if reduced fractions were
restored to ints after the gcd step:
Fraction(1*10**18, 3*10**18)
Fraction(1L, 3L)
___
Python tracker [EMAIL PROTECTED]
Guido van Rossum [EMAIL PROTECTED] added the comment:
+1 on removing the trailing L from the repr.
+0 on trying to reduce the values to ints; that would be dead-end code
since in 3.0 it's a non-issue. And it doesn't solve the problem with repr.
___
Python
Jeffrey Yasskin added the comment:
I agree that we're basically done here. I'm back to -1 on inlining the
common case for arithmetic (attached here anyway). Simple cases are
already pretty fast, and bigger fractions are dominated by gcd time, not
function call overhead. Since duplicating the
Mark Dickinson added the comment:
I'm wondering what there is left to do here. Are we close to being able
to close this issue? Some thoughts:
(1) I think the docs could use a little work (just expanding, and giving
a few examples). But it doesn't seem urgent that this gets done before
the
Mark Dickinson added the comment:
Okay. I'll stop now :)
So I lied. In a spirit of enquiry, I implemented math.gcd, and wrote a
script (lehmer_gcd.py, attached) to produce some relative timings. Here
are the results, on my MacBook Pro (OS X 10.5):
An explanation of the table: I'm
Mark Dickinson added the comment:
And here's the diff for math.gcd. It's not production quality---it's just
there to show what's possible.
Added file: http://bugs.python.org/file9486/lehmer_gcd.patch
__
Tracker [EMAIL PROTECTED]
Guido van Rossum added the comment:
A C reimplementation of gcd probably makes little sense -- for large
numbers it is dominated by the cost of the arithmetic, and that will
remain the same.
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
Mark Dickinson added the comment:
A C reimplementation of gcd probably makes little sense -- for large
numbers it is dominated by the cost of the arithmetic, and that will
remain the same.
That's true for a direct translation of the usual algorithm. But
there's a variant due to Lehmer
Guido van Rossum added the comment:
I think this is definitely premature optimization. :-)
In any case, before going any further, you should design a benchmark
and defend it.
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
Changes by Mark Dickinson:
Removed file: http://bugs.python.org/file9463/lehmer_gcd.py
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
__
___
Python-bugs-list mailing list
Jeffrey Yasskin added the comment:
1) No worries. Even without inlining the common case of __add__, etc.,
Fraction is now faster than Decimal for smallish fractions
[sum(Fraction(1, i) for i in range(1, 100))], and for large ones
[sum(Fraction(1, i) for i in range(1, 1000))] gcd takes so much of
Guido van Rossum added the comment:
Yay! And my benchmark went from 70 usec to 15 usec. Not bad!
PS. Never use relative speedup numbers based on profiled code -- the
profiler skews things tremendously. Not saying you did, just stating
the obvious. :)
__
Tracker
Guido van Rossum added the comment:
PS. I can shave off nearly 4 usec of the constructor like this:
-self = super(Fraction, cls).__new__(cls)
+if cls is Fraction:
+self = object.__new__(cls)
+else:
+self = super().__new__(cls)
This would seem to
Jeffrey Yasskin added the comment:
Thanks for writing the __add__ optimization down. I hadn't realized how
simple it was. I think both optimizations are worth it. 22% on a rarely
used class is worth 24 lines of python, and I think nearly eliminating
the overhead of ABCs (which will prevent them
Jeffrey Yasskin added the comment:
r60785 speeds the benchmark up from 10.536s to 4.910s (on top of
whatever my __instancecheck__ fix did). Here are the remaining
interesting-looking calls:
ncalls tottime percall cumtime percall filename:lineno(function)
...
10.2070.207
Nick Coghlan added the comment:
Thanks for adding the class methods back Mark.
On the constructor front, we got a fair payoff in the early Decimal days
just by reordering the type checks in the constructor. Other than that,
I'd have to dig into the code a bit more to offer any useful
Mark Dickinson added the comment:
BTW I think the next goal should be to reduce the cost of constructing
a Fraction out of to plain ints by at least an order of magnitude. I
believe this is possible.
I certainly hope so! Here's a very simple (and simplistic) benchmark:
# start benchmark
Mark Dickinson added the comment:
limit_denominator implemented in r60752
from_decimal and from_float restored to classmethods in r60754
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
__
Guido van Rossum added the comment:
Okay, Nick; you've got me convinced. For some reason I wasn't really
thinking of these methods as alternative constructors---in this light,
your arguments about doing the same as __new__, and about established
patterns, make perfect sense.
Looking back
Mark Dickinson added the comment:
Okay, Nick; you've got me convinced. For some reason I wasn't really
thinking of these methods as alternative constructors---in this light,
your arguments about doing the same as __new__, and about established
patterns, make perfect sense.
Looking back at
Mark Dickinson added the comment:
Nick Coghlan wrote:
I mentioned my dislike of the classmethod-staticmethod change on the
python-checkins list, but given the lack of response there, I figure I
should bring it up here.
Yes, I missed it. Apologies. I'll rethink this (and likely-as-not
Nick Coghlan added the comment:
I mentioned my dislike of the classmethod-staticmethod change on the
python-checkins list, but given the lack of response there, I figure I
should bring it up here. It is well established that the way to
implement an alternate constructor in Python is to write a
Jeffrey Yasskin added the comment:
Re convergents: In the interest of minimality, I don't think
from_continued_fraction and to_continued_fraction need to stick around.
I think the other thread established pretty conclusively that
.convergents() is not a particularly good building block for
Mark Dickinson added the comment:
Name change in r60721.
--
title: Move Demo/classes/Rat.py to Lib/rational.py and fix it up. - Move
Demo/classes/Rat.py to Lib/fractions.py and fix it up.
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1682
Mark Dickinson added the comment:
I just checked in another very minor change in r60723. The repr of a
Fraction now looks like Fraction(1, 2) instead of Fraction(1,2). Let me
know if this change is more controversial than I think it is.
__
Tracker [EMAIL
34 matches
Mail list logo
