classification
Title: Decimal constants should be the same for py & c module versions
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: skrah Nosy List: asvetlov, mark.dickinson, python-dev, skrah, yselivanov
Priority: normal Keywords: patch

Created on 2012-11-06 23:04 by yselivanov, last changed 2013-01-16 12:15 by skrah. This issue is now closed.

Files
File name Uploaded Description Edit
issue16422.diff skrah, 2013-01-15 12:31 review
Messages (12)
msg175024 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2012-11-06 23:04
Right now decimal.py defines 'ROUND_DOWN' as 'ROUND_DOWN' (string), whereas C version define it as 1 (integer).

While using constant values directly in your code is not a good idea, there is another case where it doesn't work: if you serialize decimal values along with their meta information, such as rounding settings.  In this case, when you unserialize data that was serialized with python decimal, those meta-settings won't work for 'c' decimal.
msg175027 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-11-06 23:18
I think it's already true that pickling a Decimal context on Python 3.2 and unpickling on Python 3.3 doesn't work.  Stefan:  do I recall that this is a known issue?
msg175029 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-11-06 23:30
Pickling changed in 3.3 to make the C and Python versions compatible. So pickled
contexts in 3.3 are actually interchangeable.

Pickling a list of rounding modes is not compatible.
msg175030 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2012-11-06 23:32
Well, I don't care about py 3.2 & 3.3 pickle compatibility in this particular issue.  This one is about compatibility of py & c decimal modules in 3.3.
msg175032 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-11-06 23:35
So what data structure are you trying to serialize interchangeably?
msg175035 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-11-06 23:41
I see that you mentioned the use case in your first mail. Yes, that isn't
possible right now.
msg175036 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2012-11-06 23:43
Right ;)

Is there any chance we can fix that in next 3.3 point release or 3.4?
msg175058 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-11-07 09:36
It would be possible to translate strings to integers; the infrastructure
is already there for pickling. The decision not to do that was actually
deliberate: Up to now no one has requested string constants for rounding
modes and I *suspect* that there's a performance penalty even though I
didn't measure it.


See Modules/_decimal/_decimal.c:1211 for the code that would need to
be called each time something like this occurs:

  context.rounding = ROUND_UP
msg175122 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-11-07 20:14
Well, I suppose one could use a cascaded switch statement, starting with the
7th letter and then (in the case of HALF*) proceed with the 12th letter:

"ROUND_CEILING"
"ROUND_FLOOR"
"ROUND_UP"
"ROUND_DOWN"
"ROUND_HALF_UP"
"ROUND_HALF_DOWN" 
"ROUND_HALF_EVEN"
"ROUND_05UP"

That should be as fast as PyLong_AsLong().
msg180013 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-01-15 12:31
In the absence of an enum type, string constants are nicer to read in
the REPL, so here's a patch. Translating between strings and ints is
fast if you use the module constants.
msg180080 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-16 12:02
New changeset 733bb4fd9888 by Stefan Krah in branch '3.3':
Issue #16422: Use strings for rounding mode constants for better readability
http://hg.python.org/cpython/rev/733bb4fd9888
msg180085 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2013-01-16 12:15
In the version I committed the string constants are interned, so
anything but legacy strings should be reasonably fast.
History
Date User Action Args
2013-01-16 12:15:42skrahsetstatus: open -> closed
messages: + msg180085

assignee: skrah
components: + Extension Modules
resolution: fixed
stage: resolved
2013-01-16 12:02:10python-devsetnosy: + python-dev
messages: + msg180080
2013-01-15 12:32:00skrahsetfiles: + issue16422.diff
keywords: + patch
messages: + msg180013
2012-11-15 15:48:34asvetlovsetnosy: + asvetlov
2012-11-07 20:14:33skrahsetmessages: + msg175122
2012-11-07 09:36:11skrahsetmessages: + msg175058
2012-11-06 23:43:06yselivanovsetmessages: + msg175036
2012-11-06 23:41:14skrahsetmessages: + msg175035
2012-11-06 23:35:28skrahsetmessages: + msg175032
2012-11-06 23:32:57yselivanovsetmessages: + msg175030
2012-11-06 23:30:59skrahsetmessages: + msg175029
2012-11-06 23:18:56mark.dickinsonsetmessages: + msg175027
2012-11-06 23:10:29mark.dickinsonsetnosy: + mark.dickinson
2012-11-06 23:04:32yselivanovcreate