[C++] Kernel to return Array of elements at index of list in ListArray

[asfimport]

It would be useful to have a compute function that takes a ListArray<list<type>> and an integer index n and returns an Array<type> containing the n th item from each list.

This would be useful in combination with existing functions that return list-type output, for example the string splitting functions.

Let's please ensure that this also works on fixed size list.

Reporter: Ian Cook / @ianmcook
Assignee: Percy Camilo Triveño Aucahuasi / @aucahuasi

Related issues:

• [R] Binding for median() and quantile() aggregation functions (blocks)

PRs and other links:

• GitHub Pull Request #11159

_{Note: This issue was originally created as ARROW-12669. Please see the migration documentation for further details.}

[asfimport]

Eduardo Ponce / @edponce:
IIUC, the expected behavior in Numpy is:

>>> listarray = np.array([[1,2],[3,4],[5,6]])
array([[1, 2],
       [3, 4],
       [5, 6]])
>>> array = listarray[:,0]  # Get index 0 from all arrays
array([1, 3, 5])
>>> array = listarray[:,1]  # Get index 1 from all arrays
array([2, 4, 6])

It could be similar to the values_slice but at the ListArray level.

[asfimport]

Percy Camilo Triveño Aucahuasi / @aucahuasi:
Should we crash if we have an out of bound index? cc @ianmcook  @edponce

[asfimport]

Neal Richardson / @nealrichardson:
We should never crash, should we?

[asfimport]

Eduardo Ponce / @edponce:
By convention, runtime errors trigger/return an error status. Status codes are listed in status.h.
For this case, I suggest something along the lines of return Status::Invalid("index out of bounds ...").

If the question is about maybe returning an empty array when an invalid index is given, I will state that most programming languages will emit an index error for such cases. I would not recommend returning an empty array via a silent error.

[asfimport]

Percy Camilo Triveño Aucahuasi / @aucahuasi:
I like the idea of returning Status::Invalid. Thanks!

More questions:

1. What is the best name for this new function? Given that this function is under Structural transforms category, my suggestion is list_nth_indices

2. If some item in each list is null: Do we need to have an option (skip_null_values) to handle this? For instance: **

   list_nth_indices with the array [[2,13],[4,null]] and the index 1
   Returns:
   - [13, null] if skip_null_values = False
   - [13] if skip_null_values = True

3. If any list is null, do we need an option to handle the output? For instance using skip_null_lists 

   list_nth_indices with the array [[2,13],null,[4,null]] and the index 1
   Returns:
   - [13, null, null] if skip_null_lists = False and skip_null_values = False
   - [13, null] if skip_null_lists = False and skip_null_values = True
   - [13, null] if skip_null_lists = True and skip_null_values = False
   - [13] if skip_null_lists = True and skip_null_values = True 

[asfimport]

Eduardo Ponce / @edponce:
For the current requirement, the output array of list_nth_indices should be of the same length as the number of arrays.

>>> list_nth_indices([[2, 13], [4, null]], /*index=*/1)
[13, null]
>>> list_nth_indices([[2, 13], null, [4, null]], /*index=*/1)
[13, null, null]

I think that if adding the skip_nulls option is not difficult to implement and its impact in performance is negligible then supporting it is not a bad idea since already other operations on ListArray skip nulls (e.g., flatten)

[asfimport]

Neal Richardson / @nealrichardson:
IMO we should always return an Array the same length as the ListArray. I don't see a use case where I would want the resulting array to be a different length. This should also simplify things because doesn't that make this kernel a scalar kernel?

If you want to drop nulls, there are ways to do that after the fact (see also ARROW-12596).

[asfimport]

Ian Cook / @ianmcook:
Regarding nulls: I agree with Neal's assertion directly above.

Regarding the name: I think list_nth_indices is confusing for several reasons. Can we name it list_element? I think we should avoid using "nth" in the name because it might cause users to believe this was related to std::nth_element or to the partition_nth_indices kernel.

[asfimport]

Percy Camilo Triveño Aucahuasi / @aucahuasi:
Sounds good! I like better that name, thanks all!

btw the PR is here (still WIP) #11159

[asfimport]

Joris Van den Bossche / @jorisvandenbossche:
Agreed with @ianmcook that list_nth_indices is a confusing name (especially the "indeces", since you are not querying indices). list_element sounds good, a potentially even more explicit (but longer) name could be list_get_element (in pandas this corresponds to Series.str.get(..), although for string values, not lists).

Since we might want to have other variations like getting a slice of each list (list_slice?) or a sublist of each list based on indices (list_elements?), it's good to have a now that can consistently be extended like this.

[asfimport]

Percy Camilo Triveño Aucahuasi / @aucahuasi:
Thanks! Another couple of questions:

1. Do we need this function can support scalar inputs?

2. if so, can you provide some examples?

   cc @ianmcook  

[asfimport]

David Li / @lidavidm:
A scalar kernel should support scalar inputs. I don't think it's a lot of work here, and it makes the testing uniform instead of having to special-case this function.

The behavior should follow from the array behavior. That is, given a (scalar) list and a (scalar) index, return the element at the index. Return null if the (scalar) list is null.

[asfimport]

Joris Van den Bossche / @jorisvandenbossche:
Yes, to confirm what David said, I expect for any scalar kernel to just work on a scalar input as well (this is basically the definition of a scalar kernel, in that the operation is done element-wise for each element independently, so naively you can see a scalar kernel as an operation on a scalar that you apply on each element of the array).

And I think David gave a concrete example here: #11159 (comment)

[asfimport]

Percy Camilo Triveño Aucahuasi / @aucahuasi:
Thanks all! Yes I've implemented the new scalar function to work with scalars and arrays as well.

The PR should be merged soon (y)

[asfimport]

David Li / @lidavidm:
Issue resolved by pull request 11159
#11159