edponce commented on a change in pull request #12660:
URL: https://github.com/apache/arrow/pull/12660#discussion_r831627984
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -898,10 +898,16 @@ class RoundTemporalOptions(_RoundTemporalOptions):
cdef class _RoundToMultipleOptions(FunctionOptions):
def _set_options(self, multiple, round_mode):
- self.wrapped.reset(
- new CRoundToMultipleOptions(multiple,
- unwrap_round_mode(round_mode))
- )
+ if isinstance(multiple, Scalar):
+ self.wrapped.reset(
+ new CRoundToMultipleOptions(pyarrow_unwrap_scalar(multiple),
+ unwrap_round_mode(round_mode))
+ )
+ else:
+ self.wrapped.reset(
+ new CRoundToMultipleOptions(<double> multiple,
+ unwrap_round_mode(round_mode))
+ )
Review comment:
After testing and investigating further, I consider the above code is
sufficient and is consistent with the other `FunctionOptions` definitions. In
the current state, any non-scalar `multiple` that can be casted to `<double>`
is valid (e.g., integers, floats, bools). Those values that cannot be coerced
trigger the following Python error:
```python
>>> pc.round_to_multiple(2.1, options=pc.RoundToMultipleOptions('one'))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pyarrow/_compute.pyx", line 930, in
pyarrow._compute.RoundToMultipleOptions.__init__
File "pyarrow/_compute.pyx", line 908, in
pyarrow._compute._RoundToMultipleOptions._set_options
TypeError: must be real number, not str
>>> asin('one')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: must be real number, not str
```
This is the same behavior as the C++ API which has a constructor with a
`double` parameter. Python, C++, and R allow casting boolean to floating-point.
Also, the above `_set_options()` follows the same pattern as all other
`FunctionOptions` (and C++) where no type checking in addition to the
language's is performed. I do not think we need tests for this one case only.
cc @lidavidm @jorisvandenbossche
--
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]
