Repository navigation
Make Context._clamp public in decimal module #52786
Description
| BPO | 8540 |
|---|---|
| Nosy | @rhettinger, @facundobatista, @mdickinson, @skrah |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/mdickinson'
closed_at = <Date 2010-05-22.18:36:28.747>
created_at = <Date 2010-04-26.17:45:30.022>
labels = ['type-feature', 'library']
title = 'Make Context._clamp public in decimal module'
updated_at = <Date 2011-10-24.09:33:52.040>
user = 'https://github.com/mdickinson'bugs.python.org fields:
activity = <Date 2011-10-24.09:33:52.040>
actor = 'mark.dickinson'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2010-05-22.18:36:28.747>
closer = 'mark.dickinson'
components = ['Library (Lib)']
creation = <Date 2010-04-26.17:45:30.022>
creator = 'mark.dickinson'
dependencies = []
files = ['17438']
hgrepos = []
issue_num = 8540
keywords = ['patch']
message_count = 15.0
messages = ['104263', '104306', '104307', '104308', '104310', '104322', '104344', '104504', '106290', '106292', '106293', '106300', '106305', '106320', '146285']
nosy_count = 4.0
nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'skrah']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = 'test needed'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue8540'
versions = ['Python 3.2']
Activity
mdickinson commentedon Apr 26, 2010
The Context class in the decimal module has a hidden _clamp attribute, that controls whether to clamp values with large exponents. (Clamping a Decimal value involves adding extra significant zeros to decrease its exponent, while not altering the numeric value; it's sometimes necessary to get the exponent within a legal range).
I think we should consider making this attribute public (documenting it, having it appear in the __str__ or __repr__ of a Context), for a couple of reasons:
(1) It's necessary for full compliance with the specification: Python's default behaviour is actually non-compliant with the spec; it's only after doing getcontext()._clamp = 1 that it complies.
(2) Clamping is necessary for modeling the standard formats described in IEEE 754-2008 (decimal64, decimal128), etc. These formats are coming into use in other languages (gcc already supports them and C201x may well include them). To be able to communicate effectively with other languages using these formats, it would be useful to expose Context._clamp.
skrah commentedon Apr 27, 2010
(1) Perhaps I missed the relevant part in the spec, so I had to check
what decNumber does: In the default context, clamp is 0, in the extended
contexts, it is 1. So Python's ExtendedContext should indeed set _clamp
to 1.
(2) I agree about the importance of the formats, and I think they should
be explicitly selectable like here:
http://java.sun.com/j2se/1.5.0/docs/api/java/math/class-use/MathContext.html
The current ExtendedContext is a bit of a vague concept, since it
should be exactly one of DECIMAL32, DECIMAL64 or DECIMAL128. And
none of these has prec=9.
skrah commentedon Apr 27, 2010
I knew it was somewhere: In the test case description, clamp=0 is
specified as the default:
http://speleotrove.com/decimal/dtfile.html#syntax
mdickinson commentedon Apr 27, 2010
Hmm. Yes, the spec itself is rather vague on the subject of clamping, so I withdraw my claim that clamping is necessary for compliance with the spec. It *is* necessary to make the those testcases with 'clamp=1' pass, though.
So from the point of view of compliance with the specification it's fine to leave the _clamp attribute private.
I agree it would be useful to give easy access to the IEEE 754 formats, though, for interoperability with C and Java (thanks for the Java link!). The most important formats are decimal32, decimal64 and decimal128, but IEEE 754 also specifies parameters for decimal{k} where k is any multiple of 32 bits.
mdickinson commentedon Apr 27, 2010
Re: ExtendedContext, the comments in decimal.py say:
# Pre-made alternate contexts offered by the specification
# Don't change these; the user should be able to select these
# contexts and be able to reproduce results from other implementations
# of the spec.
This doesn't make a lot of sense to me, since (as Stefan says) the choice of precision 9 doesn't seem to come from the specification.
However, the current ExtendedContext is used extensively in doctests and elsewhere; it would be awkward to change it now. I think adding the IEEE formats and encouraging users to use those in place of ExtendedContext might be the best bet.
I'd also still want to make _clamp public in this case, to avoid people wondering why two apparently identical contexts (one with _clamp=1, one with _clamp=0) can lead to different results.
skrah commentedon Apr 27, 2010
I'd prefer to drop the ExtendedContext completely. Reasons are:
_clamp, prec, emin and emax aren't set to IEEE754 values.
The use of 'extended' is decNumber specific (see
http://speleotrove.com/decimal/dncont.html ). In IEEE754
'extended' has yet another meaning (AFAICS).
I can see that it is awkward to remove it, but if there's consensus
I'd be willing to work on a patch.
Making clamp visible sounds fine to me. (Personally, I'd rather have
capitals non-visible.)
If we make Decimal{32,64,128} contexts available, we should document exactly how the arithmetic deviates from IEEE754.
mdickinson commentedon Apr 27, 2010
We have to be careful not to break existing 3rd party code, though. A newly-designed decimal module, prepared with 20/20 hindsight, probably wouldn't include an ExtendedContext with the current definition. But we're not dealing with a newly-designed module: we're dealing with a well-established and well-used module, so there's not a lot of scope for removals or major behaviour changes.
Personally, I don't think the design error (if there is a design error here) is big enough to make it worth breaking backwards compatibility. I'd rather leave ExtendedContext in for the duration of Python 3.x, but introduce better options and steer (via the documentation) new decimal users towards those.
(BTW, Python takes backwards compatibility pretty seriously: see PEP-387 for a write-up of the policy.)
Possibly, though that's less important to me than just being able to read and write values in these formats produced by other systems.
Were you thinking of any deviations in particular?
Making clamp public should be straightforward enough though, and is independent of the changes discussed above; I'll see if I can come up with a patch. (Even here, though, I think it makes sense to leave the private _clamp attribute in place in case people are using it; a new public 'clamp' property can be added that wraps the _clamp attribute.)
skrah commentedon Apr 29, 2010
More or less a commentary on what is listed here:
http://speleotrove.com/decimal/dascope.html
==> Restrictions
I only have a draft of IEEE754, but for instance I can't find a power
or remainder-near function there.
skrah commentedon May 22, 2010
I'm busy implementing the IEEE754 contexts for cdecimal. To keep things
in sync, it would be nice to agree how they should be created.
Suggestions:
c = Decimal64Context
c = Context(Decimal64)
?
I have a preference for 2). It's clear that you get a new Object and
the user does not have to wonder if a template context will be
contaminated when using setcontext(Decimal64Context).
(I know there are measures against that, but setcontext( Context(Decimal64))
is explicit rather than implicit.)
mdickinson commentedon May 22, 2010
Rather that complicating the Context constructor, I'd prefer a separate factory function. I was thinking of something like:
Again, it's clear with this that you get a new context object (I agree that there are problems with (1) and the mutability of Contexts).
mdickinson commentedon May 22, 2010
BTW, let's open another issue for support of the IEEE 754 contexts, and keep this one for exposing _clamp. Otherwise life gets confusing when you're trying to decide when an issue can be closed, etc...
mdickinson commentedon May 22, 2010
Here's a patch for changing '_clamp' into 'clamp'.
skrah commentedon May 22, 2010
The patch looks good, +1 for applying it. I'm not a native speaker, but
probably:
"are subject to clamping this manner" => "are subject to clamping in this manner"
mdickinson commentedon May 22, 2010
Thanks for catching the doc typo! Actually, I didn't like that doc addition anyway, so I rewrote it.
Committed in r81476.
mdickinson commentedon Oct 24, 2011
New changeset 221638ba5d2a by Mark Dickinson in branch 'default':
Issue bpo-13248, issue bpo-8540: Remove deprecated Context._clamp attribute from Decimal module.
http://hg.python.org/cpython/rev/221638ba5d2a