[Python-Dev] Decimal & returning NotImplemented (or not)

[Nick Coghlan]

Guido van Rossum wrote:
> No, the reason is that if we did this with exceptions, it would be
> liable to mask errors; an exception does not necessarily originate
> immediately with the code you invoked, it could have been raised by
> something else that was invoked by that code. The special value
> NotImplemented must pretty much originate directly in the invoked
> code, since the implementation guarantees that e.g. a+b can never
> *return* NotImplemented: if a.__add__(b) and b.__radd__(a) both return
> NotImplemented, TypeError is raised.

That makes sense - although for that reasoning, a TypeError subclass that the 
binary operation machinery promotes to a standard TypeError would seem to work 
too (with the advantage of also raising at least some sort of exception when the 
method is called directly)

> You went on to great lengths later about how it's less natural in
> Python to return an error value, but I disagree. I've written a lot of
> code like this and it's quite natural to write things like this:
> 
>     def __add__(self, other):
>         if not isinstance(other, ThisClass):
>             return NotImplemented
>         return ...implementation of self + other...

It wasn't so much this part that struck me as ugly, as the subsequent usage of 
the methods directly in the Decimal Context implementation.

The inconsistency of the interface was the main irritation. All of the methods 
that implemented standard binary operations returned NotImplemented on an 
argument error, while other methods raised TypeError directly. So for some 
methods Context was checking the return value and raising TypeError, but for 
others it was just making the call. The mixture of the two styles didn't look 
nice :)

> If you want to factor out the type check, it makes just as much sense
> to define a Boolean function:
> 
>     def __add__(self, other):
>         if not self.acceptableArgument(other):
>             return NotImplemented
>         ...etc...

I'm considering an approach that involves the simple wrapper function:

def raiseIfNotImplemented(func, message):
   def wrapper(*args, **kwds):
     result = func(*args, **kwds)
     if result is NotImplemented:
         raise TypeError(message)
     return result

After rewriting _convert_other and all the binary special methods to return 
NotImplemented for bad arguments (as you suggest), the wrapper above can be used 
to provide alternate functions that raise the TypeError instead (making it easy 
to provide a consistent API for use by Context).

Obviously, such a strategy makes this a 2.5 only solution, as it involves adding 
methods. A potentially-2.4-compatible solution is the one I used in 'friendly 
decimal' which simply duplicates the code of the above wrapper wherever it is 
needed by Decimal or Context.

> I'm guessing this wasn't caught before 2.4 was released because most
> people reviewing the code were more interested in correct computations
> than into correct integration in the Python framework.

This is quite likely to be true :)

Although I find it interesting that string objects share the same characteristic 
of not respecting __rop__ when it is provided by another class that is not a 
subclass of string.

Regards,
Nick.

-- 
Nick Coghlan   |   ncoghlan at email.com   |   Brisbane, Australia
---------------------------------------------------------------
             http://boredomandlaziness.skystorm.net