Re: [Numpy-discussion] Usage of NotImplemented in Numexpr

[Tim H. <tim...@ie...>]

Ivan Vilata i Balaguer wrote:
> Looking at the ``ophelper()`` decorator in the ``expressions`` module of
> Numexpr, I see the following code is used to check/replace arguments of
> operators::
>
>     for i, x in enumerate(args):
>         if isConstant(x):
>             args[i] = x = ConstantNode(x)
>         if not isinstance(x, ExpressionNode):
>             return NotImplemented
>
> This looks like operations on unknown kinds of arguments use the default
> Python behaviour.  However, this yields some strange results:
>
>   
>>>> import numpy
>>>> a = numpy.array([1,1,1])
>>>> import numexpr
>>>> numexpr.evaluate('a < [0,0,0]')
>>>>         
> array(True, dtype=bool)
>
> This is odd because the comparison was not element-wise, but object-wise
> (between a VariableNode and an -unsupported- python list).  Since
> Numexpr only understands scalar constants, variables and some functions
> (the last two are expression nodes), it seems more correct to me to
> simply forbid unsupported objects to avoid surprises, so the previous
> code may look like this::
>
>     for i, x in enumerate(args):
>         if isConstant(x):
>             args[i] = ConstantNode(x)
>         elif not isinstance(x, ExpressionNode):
>             raise TypeError( "unsupported object type: %s",
>                              type(x) )
>
> Do you think this is OK, or am I wrong or missing something?

That looks right. I'm not entirely happy with this fix; I believe that 
returning NotImplemented was intentional with the idea that we might 
someday want to use the NotImplemented machinery. That said, I can't 
think of a better fix and I don't see us using the NotImplemented 
machinery anytime soon, so I imagine it should go in.

-tim