Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #21153 > unrolled thread

A possible change to decimal.Decimal?

Back to article view | Back to comp.lang.python

Contents

  A possible change to decimal.Decimal? Jeff Beardsley <jbeard565@gmail.com> - 2012-03-02 14:46 -0800
    Re: A possible change to decimal.Decimal? Ethan Furman <ethan@stoneleaf.us> - 2012-03-02 15:49 -0800
      Re: A possible change to decimal.Decimal? "A. Lloyd Flanagan" <A.Lloyd.Flanagan@gmail.com> - 2012-03-04 04:37 -0800
      Re: A possible change to decimal.Decimal? "A. Lloyd Flanagan" <A.Lloyd.Flanagan@gmail.com> - 2012-03-04 04:37 -0800
        Re: A possible change to decimal.Decimal? Ethan Furman <ethan@stoneleaf.us> - 2012-03-04 07:38 -0800
    Re: A possible change to decimal.Decimal? Ethan Furman <ethan@stoneleaf.us> - 2012-03-04 07:44 -0800

#21153 — A possible change to decimal.Decimal?

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())


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)

[toc] | [next] | [standalone]

#21154

Jeff Beardsley wrote:
> 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~

[toc] | [prev] | [next] | [standalone]

#21191

On Friday, March 2, 2012 6:49:39 PM UTC-5, Ethan Furman wrote:
> Jeff Beardsley wrote:
> > HISTORY:  
...
> 
> 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.

[toc] | [prev] | [next] | [standalone]

#21194

On Friday, March 2, 2012 6:49:39 PM UTC-5, Ethan Furman wrote:
> Jeff Beardsley wrote:
> > HISTORY:  
...
> 
> 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.

[toc] | [prev] | [next] | [standalone]

#21202

A. Lloyd Flanagan wrote:
> On Friday, March 2, 2012 6:49:39 PM UTC-5, Ethan Furman wrote:
>> Jeff Beardsley wrote:
>>> HISTORY:  
> ...
>> 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.

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~

[toc] | [prev] | [next] | [standalone]

#21203

Jeff Beardsley wrote:
> 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~

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web