edponce commented on a change in pull request #11147:
URL: https://github.com/apache/arrow/pull/11147#discussion_r708416214
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -844,53 +846,52 @@ class ExtractRegexOptions(_ExtractRegexOptions):
cdef class _SliceOptions(FunctionOptions):
- def _set_options(self, int64_t start, int64_t stop, int64_t step):
+ def _set_options(self, start, stop, step):
Review comment:
Yes, they were removed deliberately. The option values are passed to the
constructors of the corresponding FunctionOptions which are defined in
[includes/libarrow.pxd](https://github.com/apache/arrow/blob/master/python/pyarrow/includes/libarrow.pxd)
and expose the explicit C types. Invalid types from public functions in .pyx
get "validated" during initialization in constructors. If the types are
included in the `cdef _XXXOptions`, then inputs would be "validated" here,
basically one line of code earlier. Duplicating types in the `_FunctionOptions`
methods in `_compute.pyx` does not adds any different functionality, but
requires developers/maintainers to ensure consistency between these files and
the C++ option definitions. I think removing them from .pyx file is a better
approach.
--
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]
