Stefan Krah stefan-use...@bytereef.org added the comment:
It looks good (I agree on number_class), but I'd change these:
- Add raiseit=True to context.copy_decimal()
- Remove wrong comment from context.is_infinite()
- Add _convert_other(a, raiseit=True) to context.logical_invert()
-
Mark Dickinson dicki...@gmail.com added the comment:
Tweaked some doctests in r78218:
- add integer argument doctest for logical_invert
- fix integer literals with a leading zero for the other logical_***
methods, since they're illegal in Python 3.
Merged to py3k in r78219.
Thanks,
Mark Dickinson dicki...@gmail.com added the comment:
The latest patch looks fine. I've attached a slightly tweaked version:
- Add conversions for number_class; without this, number_class is
inconsistent with the various is_*** methods. c.is_normal(3) should be
equivalent to
Mark Dickinson dicki...@gmail.com added the comment:
The updated patch looks good---thank you! We're getting there... :)
I'm not sure about the extra 'Operand can be Decimal or int.' in the method
docstrings; this just looks like extra clutter to me. Rather, I think it
would be a surprise
Mark Dickinson dicki...@gmail.com added the comment:
copy_sing fixed and documented to have the same behaibour.
Hmm. Thanks for noticing this: it looks like Decimal.copy_sign is missing a
_convert_other call. I think that should be fixed in the Decimal class rather
than in the Context
Stefan Krah stefan-use...@bytereef.org added the comment:
I also think that the added docstrings are not really necessary.
Another thing: I forgot to mention 'canonical' in the list of functions
that should only accept Decimals. As with the other two (number_class
and is_canonical), this is a
Mark Dickinson dicki...@gmail.com added the comment:
Re: canonical. Yes, this made me pause for a second, too. But I don't see the
harm in allowing it to accept ints and longs. Actually, it then provides a
nice public version of _convert_other.
I'd probably also allow is_canonical and
Stefan Krah stefan-use...@bytereef.org added the comment:
Yes, indeed 'canonical' can be justified to take an integer, if we interpret
the spec as:
'canonical' takes an operand and returns the preferred _decimal_
encoding of that operand.
But then 'is_canonical' should return false for an
Juan José Conti jjco...@gmail.com added the comment:
Yeah... I did't like that docstring either :) Removed!
Also fixed Decimal.copy_sign, changed Context.copy_sign and added tests.
--
Added file: http://bugs.python.org/file16052/issue7633_jjconti4.patch
Juan José Conti jjco...@gmail.com added the comment:
1) Agree. Extra checks removed.
2) My mistake. Fixed.
3) Fexed.
4) Methods documentation fixed. Also added examples.
5) Fixed
6) Allow ints in the following unary methods (all except the ones excluded by
skrah in cdecimal):
- abs
- canonical
Stefan Krah stefan-use...@bytereef.org added the comment:
If none of you is working on it right now, I'll produce a new patch.
Mark, how about this:
def __add__(self, other, context=None, raiseit=False):
Returns self + other.
-INF + INF (or the reverse) cause
Juan José Conti jjco...@gmail.com added the comment:
I've been working in the modified version of my last patch to solve the 6
mentioned points. I'm posting it in less than 24 hs.
If you're not hurry, please wait for me. This is just my second patch and is
very useful to learn how to
Stefan Krah stefan-use...@bytereef.org added the comment:
Juan: Sure, take your time. :) I just wanted to know if you were still busy
with it.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue7633
Mark Dickinson dicki...@gmail.com added the comment:
Juan, don't worry about the documentation if you don't want to. I can fix that
up easily. (Of course, if you do want to, go ahead!)
--
___
Python tracker rep...@bugs.python.org
Facundo Batista facu...@taniquetil.com.ar added the comment:
Juanjo, ping me in private if you want help with the doc toolchain, I can show
you how to touch the .rst and see the changes after processing.
--
___
Python tracker rep...@bugs.python.org
Mark Dickinson dicki...@gmail.com added the comment:
Thanks for the latest patch! It's looking good, but I have a few comments:
(1) It's not necessary to do an isinstance(a, Decimal) check before calling
_convert_other, since _convert_other does that check anyway. It doesn't really
harm
Mark Dickinson dicki...@gmail.com added the comment:
One more:
(5) The patch includes a (presumably accidental) change to the divmod
docstring, from Return (a // b, a % b) to Return (self // other, self %
other). I think the first version is preferable.
--
Mark Dickinson dicki...@gmail.com added the comment:
And another. :)
(6) I think that with these changes, the single argument methods, like
Context.exp, and Context.sqrt, should also accept integers. Thoughts?
--
___
Python tracker
Stefan Krah stefan-use...@bytereef.org added the comment:
(6) Yes, I think that is logical. In cdecimal, I accept integers for all unary
methods except is_canonical and number_class.
--
___
Python tracker rep...@bugs.python.org
Mark Dickinson dicki...@gmail.com added the comment:
Thanks for the patch!
Rather than using the Decimal constructor, I think you should convert use
_convert_other(..., raiseit=True): the Decimal constructor converts strings
and tuples, as well as ints and longs, while _convert_other only
Mark Dickinson dicki...@gmail.com added the comment:
Thanks for the missing divmod docstring, too: I've applied this separately,
partly because it needs to go into 2.6 and 3.1 as well as 2.7 and 3.2, and
partly as an excuse to check that the new py3k-cdecimal branch is set up
correctly. See
Juan José Conti jjco...@gmail.com added the comment:
New patch.
Fix Context.method that were returning NotImplemented.
Decimal.__methods__ don't use raiseit=True in _convert_other so I check in
Context.method and raise TypeError if necessary.
Added tests for Context.divmod missed in first
Juan José Conti jjco...@gnu.org added the comment:
The same happens with other Context methods, like divide:
Python 2.6.2 (release26-maint, Apr 19 2009, 01:56:41)
[GCC 4.3.3] on linux2
Type help, copyright, credits or license for more information.
from decimal import *
c = getcontext()
Mark Dickinson dicki...@gmail.com added the comment:
I agree that this would be desirable. And yes, c.add(9, 8) should be allowed,
too.
Anyone interested in producing a patch?
--
components: +Library (Lib)
keywords: +easy
nosy: +facundobatista, rhettinger
priority: - normal
stage:
Juan José Conti jjco...@gmail.com added the comment:
I've been reading http://speleotrove.com/decimal and seems not to be an
explicit definition about this. I'm thinking in a patch I could supply tomorrow.
What about the idea of changing the implementation from:
return a.__add__(b,
Mark Dickinson dicki...@gmail.com added the comment:
What about the idea of changing the implementation from:
return a.__add__(b, context=self)
to
return Decimal(a+b,context=self)
?
I think it would be better to convert the arguments a and b to Decimal before
doing the
Juan José Conti jjco...@gmail.com added the comment:
The attached patch solves this issue.
Finally, only operand 'a' needs to be converted to Decimal if it's not. I
discover this after writing my tests and start the implementation.
A Context.method is modified if it has more than one operand
New submission from Stefan Krah stefan-use...@bytereef.org:
I think that context methods should convert arguments regardless of position:
Python 2.7a0 (trunk:76132M, Nov 6 2009, 15:20:35)
[GCC 4.1.3 20080623 (prerelease) (Ubuntu 4.1.2-23ubuntu3)] on linux2
Type help, copyright, credits or
28 matches
Mail list logo
