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
|
