Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(273)

#7652: Merge C version of decimal into py3k.

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 5 months ago by dickinsm
Modified:
5 years, 8 months ago
Reviewers:
amauryfa, stefan-usenet, jimjjewett
CC:
lemburg, rhettinger, amaury.forgeotdarc, mark.dickinson, AntoinePitrou, haypo, casevh_gmail.com, eric.smith, Benjamin Peterson, jjconti_gmail.com, Arfrever, ced, asvetlov, skrah, amaury.forgeotdarc, devnull_psf.upfronthosting.co.za, eric.snow, Ramchandra Apte, Jim.J.Jewett
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 87

Patch Set 5 #

Patch Set 6 #

Patch Set 7 #

Patch Set 8 #

Patch Set 9 #

Patch Set 10 #

Patch Set 11 #

Patch Set 12 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/decimal.rst View 1 chunk +6 lines, -0 lines 0 comments Download
Doc/library/numeric.rst View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -3 lines 0 comments Download
Include/longintrepr.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
Lib/decimal.py View 1 2 3 4 5 6 7 8 9 10 11 16 chunks +134 lines, -29 lines 0 comments Download
Lib/test/support.py View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
Lib/test/test_decimal.py View 1 2 3 4 5 6 7 8 9 10 11 49 chunks +3406 lines, -982 lines 0 comments Download
Lib/test/test_fractions.py View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -8 lines 0 comments Download
Lib/test/test_numeric_tower.py View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
Misc/valgrind-python.supp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +13 lines, -0 lines 0 comments Download
Modules/_decimal/ISSUES.txt View 7 8 9 10 11 1 chunk +27 lines, -0 lines 0 comments Download
Modules/_decimal/README.txt View 1 chunk +46 lines, -0 lines 0 comments Download
Modules/_decimal/_decimal.c View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5497 lines, -0 lines 0 comments Download
Modules/_decimal/docstrings.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +751 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/README.txt View 1 chunk +90 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/basearith.c View 1 chunk +635 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/basearith.h View 1 chunk +213 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/bits.h View 1 chunk +192 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/constants.c View 1 chunk +132 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/constants.h View 1 chunk +83 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/context.c View 1 chunk +300 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/convolute.c View 1 chunk +174 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/convolute.h View 1 chunk +43 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/crt.c View 1 chunk +179 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/crt.h View 1 chunk +40 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/difradix2.c View 1 chunk +173 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/difradix2.h View 1 chunk +41 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/fnt.c View 1 chunk +81 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/fnt.h View 1 chunk +42 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/fourstep.c View 1 chunk +255 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/fourstep.h View 1 chunk +41 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/io.c View 1 chunk +1575 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/io.h View 1 chunk +59 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/REFERENCES.txt View 1 chunk +51 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/bignum.txt View 1 chunk +83 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/fnt.py View 1 chunk +212 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/matrix-transform.txt View 1 chunk +256 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/mulmod-64.txt View 1 chunk +127 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/mulmod-ppro.txt View 1 chunk +269 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/six-step.txt View 1 chunk +63 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/literature/umodarith.lisp View 1 chunk +692 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/memory.c View 1 chunk +270 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/memory.h View 1 chunk +44 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/mpdecimal.c View 1 chunk +7555 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/mpdecimal.h View 1 chunk +802 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/numbertheory.c View 1 chunk +132 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/numbertheory.h View 1 chunk +71 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/sixstep.c View 1 chunk +212 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/sixstep.h View 1 chunk +41 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/transpose.c View 1 chunk +276 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/transpose.h View 1 chunk +55 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/typearith.h View 1 chunk +669 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/umodarith.h View 1 chunk +650 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/vccompat.h View 1 chunk +62 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/vcdiv64.asm View 1 chunk +48 lines, -0 lines 0 comments Download
Modules/_decimal/libmpdec/vcstdint.h View 1 chunk +232 lines, -0 lines 0 comments Download
Modules/_decimal/tests/README.txt View 1 chunk +15 lines, -0 lines 0 comments Download
Modules/_decimal/tests/bench.py View 1 chunk +116 lines, -0 lines 0 comments Download
Modules/_decimal/tests/deccheck.py View 1 chunk +1100 lines, -0 lines 0 comments Download
Modules/_decimal/tests/formathelper.py View 1 chunk +344 lines, -0 lines 0 comments Download
Modules/_decimal/tests/randdec.py View 1 chunk +559 lines, -0 lines 0 comments Download
Modules/_decimal/tests/randfloat.py View 1 chunk +250 lines, -0 lines 0 comments Download
Modules/_decimal/tests/runall-memorydebugger.sh View 1 chunk +175 lines, -0 lines 0 comments Download
Modules/_decimal/tests/runall.bat View 1 chunk +121 lines, -0 lines 0 comments Download
PCbuild/_decimal.vcproj View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +743 lines, -0 lines 0 comments Download
PCbuild/pcbuild.sln View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +21 lines, -0 lines 0 comments Download
configure View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +173 lines, -2 lines 0 comments Download
configure.in View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +100 lines, -0 lines 0 comments Download
pyconfig.h.in View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +13 lines, -0 lines 0 comments Download
setup.py View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +113 lines, -0 lines 0 comments Download

Messages

Total messages: 10
amaury.forgeotdarc
Here are my comments so far, only in _decimal.c for the moment. Many style issues, ...
5 years, 11 months ago #1
stefan-usenet_bytereef.org
Thanks for all the comments. I've replied to the easier ones. Some of the remaining ...
5 years, 11 months ago #2
stefan-usenet_bytereef.org
Thanks for all the comments. I've replied to the easier ones. Some of the remaining ...
5 years, 11 months ago #3
amaury.forgeotdarc
http://bugs.python.org/review/7652/diff/2862/7305 File Modules/_decimal/_decimal.c (right): http://bugs.python.org/review/7652/diff/2862/7305#newcode54 Modules/_decimal/_decimal.c:54: #endif Actually I saw this in the recent issue13488. ...
5 years, 11 months ago #4
stefan-usenet_bytereef.org
http://bugs.python.org/review/7652/diff/2862/7295 File Lib/decimal.py (right): http://bugs.python.org/review/7652/diff/2862/7295#newcode6345 Lib/decimal.py:6345: except: Yes, indeed. http://bugs.python.org/review/7652/diff/2862/7305 File Modules/_decimal/_decimal.c (right): http://bugs.python.org/review/7652/diff/2862/7305#newcode57 Modules/_decimal/_decimal.c:57: ...
5 years, 11 months ago #5
stefan-usenet_bytereef.org
http://bugs.python.org/review/7652/diff/2862/7305 File Modules/_decimal/_decimal.c (right): http://bugs.python.org/review/7652/diff/2862/7305#newcode462 Modules/_decimal/_decimal.c:462: /******************************************************************************/ I need to get a better grasp on ...
5 years, 11 months ago #6
stefan-usenet_bytereef.org
I've addressed most issues except for the SignalDict. Also, I agree that while the current ...
5 years, 11 months ago #7
Jim.J.Jewett
Looking at Lib/decimal.py @@ -3832,11 +3839,9 @@, but carries through to @@ -3890,16 +3956,17 @@ ...
5 years, 8 months ago #8
Jim.J.Jewett
Looking at Lib/decimal.py @@ -3832,11 +3839,9 @@, but carries through to @@ -3890,16 +3956,17 @@ ...
5 years, 8 months ago #9
stefan-usenet_bytereef.org
5 years, 8 months ago #10
> Why change the order of args in the __init__ signature?  If you need to for
> compatibility, would it make sense to at least declare everything after
rounding
> (the part that is changing) keyword-only?

This is because the current repr does not match the signature:

>>> c = Context()
>>> c
Context(prec=28, rounding=ROUND_HALF_EVEN, Emin=-999999999, Emax=999999999,
capitals=1, flags=[], traps=[Overflow, DivisionByZero, InvalidOperation])
>>> Context(28, ROUND_HALF_EVEN, -999999999, 999999999, 1, [], [Overflow,
DivisionByZero, InvalidOperation])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.2/decimal.py", line 3774, in __init__
    flags = dict([(s, int(s in flags)) for s in _signals])
  File "/usr/lib/python3.2/decimal.py", line 3774, in <listcomp>
    flags = dict([(s, int(s in flags)) for s in _signals])
TypeError: argument of type 'int' is not iterable


Using keyword-only is a great idea.


> Why all the verifications in _set_signal_dict?  Wouldn't it be enough to
insist
> that k.keys() == _signals.keys()?  If you really need a dict, that should be
> mentioned in a docstring.  Also, why is it so horrible if there are extra (or
> even missing) entries?

The signals are clearly defined in the Decimal Specification. All
signals must be present, or an exception will be raised later when
the signal is accessed. Additional entries are almost certainly
a programming error that should be detected. Efficiency does not
matter in this case, since assigning a whole new signal dict is quite
a rare occurrence (if used at all).

Currently decimal.py silently accepts invalid types:

>>> c = getcontext()
>>> c.flags = []
>>> c
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.2/decimal.py", line 3790, in __repr__
    names = [f.__name__ for f, v in self.flags.items() if v]
AttributeError: 'list' object has no attribute 'items'



But PEP-399 demands that all exceptions of the C and Python
versions must match. This is not possible if in the Python
version exceptions occur later at some unspecified point.
So I added the type check that has the additional benefit of
giving a clear error message at the moment the error occurs.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7