milesgranger commented on code in PR #14395:
URL: https://github.com/apache/arrow/pull/14395#discussion_r1011538089
##########
cpp/src/arrow/compute/kernels/scalar_nested.cc:
##########
@@ -87,6 +89,206 @@ Status GetListElementIndex(const ExecValue& value, T* out) {
return Status::OK();
}
+template <typename Type, typename IndexType>
+struct ListSlice {
+ using offset_type = typename Type::offset_type;
+
+ static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
+ const auto opts = OptionsWrapper<ListSliceOptions>::Get(ctx);
+
+ // Invariants
+ if (opts.start < 0 || (opts.start >= opts.stop && opts.stop != -1)) {
+ // TODO: support start == stop which should give empty lists
+ return Status::Invalid("`start`(", opts.start,
+ ") should be greater than 0 and smaller than
`stop`(",
+ opts.stop, ")");
+ }
+ if (opts.step != 1) {
+ // TODO: support step in slicing
+ return Status::NotImplemented(
+ "Setting `step` to anything other than 1 is not supported; got
step=",
+ opts.step);
+ }
+ if (opts.stop == -1) {
+ // TODO: Support slicing to arbitrary end
Review Comment:
> If this is all ambiguous, can we use std::optional<int64_t> for the slice
stop?
Switched to this, good idea. Although it came with some challenges as it's
[not yet available in Cython
0.29](https://github.com/cython/cython/issues/3293#issuecomment-1223058101) but
I like the more direct link to how it is represented in C++.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
