A possible change to decimal.Decimal?

J

Jeff Beardsley

HISTORY:

In using python 2.7.2 for awhile on a web project (apache/wsgi web.py), I discovered a problem in using decimal.Decimal. A short search revealed thatmany other people have been having the problem as well, in their own apache/wsgi implementations (django, mostly), but I found no real solutions among the posts I read. So I did some experimentation of my own.

The following code will break unexpectedly on standard python2.7 (and earlier) because of the way that isinstance fails after reload() (which is called by both of the above web frameworks).

This is the error: TypeError("Cannot convert %r to Decimal" % value)

THE TEST CODE

import decimal
from decimal import Decimal

#this works
Decimal(Decimal())

reload(decimal)

#this fails before patching, but works fine afterwards
Decimal(Decimal())


THE SOLUTION:

So, looking into decimal.py I discovered lots if statements using isinstance, and have slightly rearranged the code inside __new__() to mostly remove their use, and for my purposes totally fixes the problem within wsgi.

I am not an official python dev, so would appreciate it if someone who *IS*could just look this over, improve it if necessary, and get it (or some variation) submitted into the library.

Below is a patch for use against python-2.7.2

PATCH:


*** decimal.py 2012-03-02 16:42:51.285964007 -0600
--- /usr/lib/python2.7/decimal.py 2012-03-02 14:36:01.238976461 -0600
***************
*** 535,608 ****
# and the Decimal constructor still deal with tuples of
# digits.

self = object.__new__(cls)

! # From a string
! # REs insist on real strings, so we can too.
! if isinstance(value, basestring):
! m = _parser(value.strip())
! if m is None:
! if context is None:
! context = getcontext()
! return context._raise_error(ConversionSyntax,
! "Invalid literal for Decimal: %r" % value)
!
! if m.group('sign') == "-":
! self._sign = 1
! else:
! self._sign = 0
! intpart = m.group('int')
! if intpart is not None:
! # finite number
! fracpart = m.group('frac') or ''
! exp = int(m.group('exp') or '0')
! self._int = str(int(intpart+fracpart))
! self._exp = exp - len(fracpart)
! self._is_special = False
! else:
! diag = m.group('diag')
! if diag is not None:
! # NaN
! self._int = str(int(diag or '0')).lstrip('0')
! if m.group('signal'):
! self._exp = 'N'
! else:
! self._exp = 'n'
! else:
! # infinity
! self._int = '0'
! self._exp = 'F'
! self._is_special = True
! return self
!
! # From an integer
! if isinstance(value, (int,long)):
! if value >= 0:
! self._sign = 0
! else:
! self._sign = 1
! self._exp = 0
! self._int = str(abs(value))
! self._is_special = False
return self

! # From another decimal
! if isinstance(value, Decimal):
self._exp = value._exp
self._sign = value._sign
self._int = value._int
self._is_special = value._is_special
return self

# From an internal working value
! if isinstance(value, _WorkRep):
self._sign = value.sign
self._int = str(value.int)
self._exp = int(value.exp)
self._is_special = False
return self

# tuple/list conversion (possibly from as_tuple())
if isinstance(value, (list,tuple)):
if len(value) != 3:
raise ValueError('Invalid tuple size in creation of Decimal '
--- 535,582 ----
# and the Decimal constructor still deal with tuples of
# digits.

self = object.__new__(cls)

! # Note about isinstance -- Decimal has been a victim of the
! # isinstance builtin failing after module reload in some
! # environments (e.g. web.py, django) under apache/wsgi, which
! # I determined to be the reason Decimal was causing so many
! # problems in my web deployment. I have re-organized the
! # following code to remove use of isinstance except on
! # native types (int, long, float, list, tuple), since those
! # seem not to break in this regard. -- jdb
!
! # First, assume it's another Decimal or similar(having _exp,
! # _sign, _int and _is_special. Obviously, having these
! # implies it's at least an attempt to represent Decimal
! try:
! self._exp = value._exp
! self._sign = value._sign
! self._int = value._int
! self._is_special = value._is_special
return self
+ except: pass

! # Or it's a float
! try:
! value = Decimal.from_float(value)
self._exp = value._exp
self._sign = value._sign
self._int = value._int
self._is_special = value._is_special
return self
+ except: pass

# From an internal working value
! try:
self._sign = value.sign
self._int = str(value.int)
self._exp = int(value.exp)
self._is_special = False
return self
+ except: pass

# tuple/list conversion (possibly from as_tuple())
if isinstance(value, (list,tuple)):
if len(value) != 3:
raise ValueError('Invalid tuple size in creation of Decimal '
***************
*** 645,661 ****
raise ValueError("The third value in the tuple must "
"be an integer, or one of the "
"strings 'F', 'n', 'N'.")
return self

! if isinstance(value, float):
! value = Decimal.from_float(value)
! self._exp = value._exp
! self._sign = value._sign
! self._int = value._int
! self._is_special = value._is_special
return self

raise TypeError("Cannot convert %r to Decimal" % value)

# @classmethod, but @decorator is not valid Python 2.3 syntax, so
# don't use it (see notes on Py2.3 compatibility at top of file)
--- 619,666 ----
raise ValueError("The third value in the tuple must "
"be an integer, or one of the "
"strings 'F', 'n', 'N'.")
return self

! # From a string, or anything representable as a string
! try:
! value = str(value)
! m = _parser(value.strip())
! if m is None:
! if context is None:
! context = getcontext()
! return context._raise_error(ConversionSyntax,
! "Invalid literal for Decimal: %r" % value)
!
! if m.group('sign') == "-":
! self._sign = 1
! else:
! self._sign = 0
! intpart = m.group('int')
! if intpart is not None:
! # finite number
! fracpart = m.group('frac') or ''
! exp = int(m.group('exp') or '0')
! self._int = str(int(intpart+fracpart))
! self._exp = exp - len(fracpart)
! self._is_special = False
! else:
! diag = m.group('diag')
! if diag is not None:
! # NaN
! self._int = str(int(diag or '0')).lstrip('0')
! if m.group('signal'):
! self._exp = 'N'
! else:
! self._exp = 'n'
! else:
! # infinity
! self._int = '0'
! self._exp = 'F'
! self._is_special = True
return self
+ except: pass

raise TypeError("Cannot convert %r to Decimal" % value)

# @classmethod, but @decorator is not valid Python 2.3 syntax, so
# don't use it (see notes on Py2.3 compatibility at top of file)
 
E

Ethan Furman

Jeff said:
HISTORY:

In using python 2.7.2 for awhile on a web project (apache/wsgi web.py), I discovered a problem in using decimal.Decimal. A short search revealed that many other people have been having the problem as well, in their own apache/wsgi implementations (django, mostly), but I found no real solutions among the posts I read. So I did some experimentation of my own.

The following code will break unexpectedly on standard python2.7 (and earlier) because of the way that isinstance fails after reload() (which is called by both of the above web frameworks).

This is the error: TypeError("Cannot convert %r to Decimal" % value)

THE TEST CODE

import decimal
from decimal import Decimal

#this works
Decimal(Decimal())

reload(decimal)

#this fails before patching, but works fine afterwards
Decimal(Decimal())

Patching decimal.py to make it work with reload() is probably not going
to happen.

What you should be doing is:

import decimal
from decimal import Decimal

reload(decimal)
Decimal = decimal.Decimal # (rebind 'Decimal' to the reloaded code)

~Ethan~
 
A

A. Lloyd Flanagan

....

What you should be doing is:

import decimal
from decimal import Decimal

reload(decimal)
Decimal = decimal.Decimal # (rebind 'Decimal' to the reloaded code)

~Ethan~

Agree that's how the import should be done. On the other hand, removing gratuitous use of isinstance() is generally a Good Thing.
 
A

A. Lloyd Flanagan

....

What you should be doing is:

import decimal
from decimal import Decimal

reload(decimal)
Decimal = decimal.Decimal # (rebind 'Decimal' to the reloaded code)

~Ethan~

Agree that's how the import should be done. On the other hand, removing gratuitous use of isinstance() is generally a Good Thing.
 
E

Ethan Furman

A. Lloyd Flanagan said:
Agree that's how the import should be done. On the other hand, removing gratuitous use of isinstance() is generally a Good Thing.

Gratuitous use? How is it gratuitous for an class to check that one of
its arguments is its own type?

class Frizzy(object):
def __add__(self, other):
if not isinstance(other, Frizzy):
return NotImplemented
do_stuff_with(self, other)

This is exactly what isinstance() is for, and this is how it is being
used in Decimal.__init__().

~Ethan~
 
E

Ethan Furman

Jeff said:
The problem with that though: I am not calling reload(), except to
recreate the error as implemented by the web frameworks.

I am also unlikely to get a patch accepted into several different
projects, where this is ONE project, and it's a simple change

Simple -- maybe.

Appropriate -- no.

It is unfortunate that those frameworks have that bug, but it is not up
to Decimal to fix it for them.

~Ethan~
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,764
Messages
2,569,564
Members
45,040
Latest member
papereejit

Latest Threads

Top