AlenkaF commented on code in PR #49833:
URL: https://github.com/apache/arrow/pull/49833#discussion_r3860290086
##########
python/pyarrow/array.pxi:
##########
@@ -2291,47 +2290,59 @@ cdef class Array(_PandasConvertible):
def __add__(self, object other):
self._assert_cpu()
- return _pc().call_function('add_checked', [self, other])
+ return _array_binop_or_notimplemented('add_checked', self, other)
def __truediv__(self, object other):
self._assert_cpu()
- return _pc().call_function('divide_checked', [self, other])
+ return _array_binop_or_notimplemented('divide_checked', self, other)
def __mul__(self, object other):
self._assert_cpu()
- return _pc().call_function('multiply_checked', [self, other])
+ return _array_binop_or_notimplemented('multiply_checked', self, other)
def __neg__(self):
self._assert_cpu()
return _pc().call_function('negate_checked', [self])
def __pow__(self, object other):
self._assert_cpu()
- return _pc().call_function('power_checked', [self, other])
+ return _array_binop_or_notimplemented('power_checked', self, other)
def __sub__(self, object other):
self._assert_cpu()
- return _pc().call_function('subtract_checked', [self, other])
+ return _array_binop_or_notimplemented('subtract_checked', self, other)
def __and__(self, object other):
self._assert_cpu()
- return _pc().call_function('bit_wise_and', [self, other])
+ return _array_binop_or_notimplemented('bit_wise_and', self, other)
def __or__(self, object other):
self._assert_cpu()
- return _pc().call_function('bit_wise_or', [self, other])
+ return _array_binop_or_notimplemented('bit_wise_or', self, other)
def __xor__(self, object other):
self._assert_cpu()
- return _pc().call_function('bit_wise_xor', [self, other])
+ return _array_binop_or_notimplemented('bit_wise_xor', self, other)
def __lshift__(self, object other):
self._assert_cpu()
- return _pc().call_function('shift_left_checked', [self, other])
+ return _array_binop_or_notimplemented('shift_left_checked', self,
other)
def __rshift__(self, object other):
self._assert_cpu()
- return _pc().call_function('shift_right_checked', [self, other])
+ return _array_binop_or_notimplemented('shift_right_checked', self,
other)
+
+
+def _array_binop_or_notimplemented(op_name, left, right):
+ # Same NotImplemented fallback as Scalar.__add__ et al, see GH-49826.
+ # Only swallow TypeError for genuinely foreign types; propagate when
+ # the right operand is already Arrow-native so type errors are visible.
Review Comment:
```suggestion
```
I think comments are not necessary here.
##########
python/pyarrow/scalar.pxi:
##########
@@ -199,37 +199,55 @@ cdef class Scalar(_Weakrefable):
return _pc().call_function('abs_checked', [self])
def __add__(self, object other):
- return _pc().call_function('add_checked', [self, other])
+ return _binop_or_notimplemented('add_checked', self, other)
def __truediv__(self, object other):
- return _pc().call_function('divide_checked', [self, other])
+ return _binop_or_notimplemented('divide_checked', self, other)
def __mul__(self, object other):
- return _pc().call_function('multiply_checked', [self, other])
+ return _binop_or_notimplemented('multiply_checked', self, other)
def __neg__(self):
return _pc().call_function('negate_checked', [self])
def __pow__(self, object other):
- return _pc().call_function('power_checked', [self, other])
+ return _binop_or_notimplemented('power_checked', self, other)
def __sub__(self, object other):
- return _pc().call_function('subtract_checked', [self, other])
+ return _binop_or_notimplemented('subtract_checked', self, other)
def __and__(self, object other):
- return _pc().call_function('bit_wise_and', [self, other])
+ return _binop_or_notimplemented('bit_wise_and', self, other)
def __or__(self, object other):
- return _pc().call_function('bit_wise_or', [self, other])
+ return _binop_or_notimplemented('bit_wise_or', self, other)
def __xor__(self, object other):
- return _pc().call_function('bit_wise_xor', [self, other])
+ return _binop_or_notimplemented('bit_wise_xor', self, other)
def __lshift__(self, object other):
- return _pc().call_function('shift_left_checked', [self, other])
+ return _binop_or_notimplemented('shift_left_checked', self, other)
def __rshift__(self, object other):
- return _pc().call_function('shift_right_checked', [self, other])
+ return _binop_or_notimplemented('shift_right_checked', self, other)
+
+
+def _binop_or_notimplemented(op_name, left, right):
+ # Scalar arithmetic dunders must return NotImplemented for argument types
Review Comment:
Would remove these verbose comments here also.
##########
python/pyarrow/tests/test_scalars.py:
##########
@@ -1022,6 +1022,16 @@ def test_bitwise_dunders():
assert (scl2 >> scl1).equals(pc.shift_right_checked(scl2, scl1))
+def test_scalar_radd_unknown_operand():
+ # GH-49826: Scalar.__add__ on an unknown right operand must return
+ # NotImplemented so Python falls back to right.__radd__.
+ class WithRadd:
+ def __radd__(self, other):
Review Comment:
Could we parametrize for other updated operands also?
##########
python/pyarrow/tests/test_array.py:
##########
@@ -4401,6 +4401,16 @@ def test_non_cpu_array():
arr.validate(full=True)
+def test_array_radd_unknown_operand():
+ # GH-49826: Array.__add__ on an unknown right operand must return
+ # NotImplemented so Python falls back to right.__radd__.
Review Comment:
```suggestion
```
Not sure these comments are needed.
##########
python/pyarrow/tests/test_scalars.py:
##########
@@ -1022,6 +1022,16 @@ def test_bitwise_dunders():
assert (scl2 >> scl1).equals(pc.shift_right_checked(scl2, scl1))
+def test_scalar_radd_unknown_operand():
+ # GH-49826: Scalar.__add__ on an unknown right operand must return
+ # NotImplemented so Python falls back to right.__radd__.
Review Comment:
```suggestion
```
Same here.
##########
python/pyarrow/array.pxi:
##########
@@ -2291,47 +2290,59 @@ cdef class Array(_PandasConvertible):
def __add__(self, object other):
self._assert_cpu()
- return _pc().call_function('add_checked', [self, other])
+ return _array_binop_or_notimplemented('add_checked', self, other)
def __truediv__(self, object other):
self._assert_cpu()
- return _pc().call_function('divide_checked', [self, other])
+ return _array_binop_or_notimplemented('divide_checked', self, other)
def __mul__(self, object other):
self._assert_cpu()
- return _pc().call_function('multiply_checked', [self, other])
+ return _array_binop_or_notimplemented('multiply_checked', self, other)
def __neg__(self):
self._assert_cpu()
return _pc().call_function('negate_checked', [self])
def __pow__(self, object other):
self._assert_cpu()
- return _pc().call_function('power_checked', [self, other])
+ return _array_binop_or_notimplemented('power_checked', self, other)
def __sub__(self, object other):
self._assert_cpu()
- return _pc().call_function('subtract_checked', [self, other])
+ return _array_binop_or_notimplemented('subtract_checked', self, other)
def __and__(self, object other):
self._assert_cpu()
- return _pc().call_function('bit_wise_and', [self, other])
+ return _array_binop_or_notimplemented('bit_wise_and', self, other)
def __or__(self, object other):
self._assert_cpu()
- return _pc().call_function('bit_wise_or', [self, other])
+ return _array_binop_or_notimplemented('bit_wise_or', self, other)
def __xor__(self, object other):
self._assert_cpu()
- return _pc().call_function('bit_wise_xor', [self, other])
+ return _array_binop_or_notimplemented('bit_wise_xor', self, other)
def __lshift__(self, object other):
self._assert_cpu()
- return _pc().call_function('shift_left_checked', [self, other])
+ return _array_binop_or_notimplemented('shift_left_checked', self,
other)
def __rshift__(self, object other):
self._assert_cpu()
- return _pc().call_function('shift_right_checked', [self, other])
+ return _array_binop_or_notimplemented('shift_right_checked', self,
other)
+
+
+def _array_binop_or_notimplemented(op_name, left, right):
+ # Same NotImplemented fallback as Scalar.__add__ et al, see GH-49826.
+ # Only swallow TypeError for genuinely foreign types; propagate when
+ # the right operand is already Arrow-native so type errors are visible.
+ try:
+ return _pc().call_function(op_name, [left, right])
+ except TypeError as e:
+ if isinstance(right, (Scalar, Array)):
+ raise
Review Comment:
```suggestion
```
I actually think the original version was enough, see
https://github.com/apache/arrow/pull/49845#discussion_r3186474132. Sorry for
that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
