[C++/Doc] The IsIn kernel ignores the skip_nulls option of SetLookupOptions

[asfimport]

The C++ docs of SetLookupOptions has this explanation of the skip_nulls option:

  /// Whether nulls in `value_set` count for lookup.
  ///
  /// If true, any null in `value_set` is ignored and nulls in the input
  /// produce null (IndexIn) or false (IsIn) values in the output.
  /// If false, any null in `value_set` is successfully matched in
  /// the input.
  bool skip_nulls;

(from

arrow/cpp/src/arrow/compute/api_scalar.h

Lines 78 to 84 in 8b9f6b9

	/// Whether nulls in `value_set` count for lookup.
	///
	/// If true, any null in `value_set` is ignored and nulls in the input
	/// produce null (IndexIn) or false (IsIn) values in the output.
	/// If false, any null in `value_set` is successfully matched in
	/// the input.
	bool skip_nulls;

)

However, for IsIn this explanation doesn't seem to hold in practice:

In [16]: arr = pa.array([1, 2, None])

In [17]: pc.is_in(arr, value_set=pa.array([1, None]), skip_null=True)
Out[17]: 
<pyarrow.lib.BooleanArray object at 0x7fcf666f9408>
[
  true,
  false,
  true
]

In [18]: pc.is_in(arr, value_set=pa.array([1, None]), skip_null=False)
Out[18]: 
<pyarrow.lib.BooleanArray object at 0x7fcf666b13a8>
[
  true,
  false,
  true
]

This documentation was added in #7695 (ARROW-8989)/
.

BTW, for "index_in", it works as documented:

In [19]: pc.index_in(arr, value_set=pa.array([1, None]), skip_null=True)
Out[19]: 
<pyarrow.lib.Int32Array object at 0x7fcf666f04c8>
[
  0,
  null,
  null
]

In [20]: pc.index_in(arr, value_set=pa.array([1, None]), skip_null=False)
Out[20]: 
<pyarrow.lib.Int32Array object at 0x7fcf666f0ee8>
[
  0,
  null,
  1
]

Reporter: Joris Van den Bossche / @jorisvandenbossche
Assignee: Antoine Pitrou / @pitrou

PRs and other links:

• GitHub Pull Request #9164

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

[asfimport]

Joris Van den Bossche / @jorisvandenbossche:
The original PR adding it seems to have documented a different behaviour at the time (https://github.com/apache/arrow/pull/4235/files#diff-fc156499f9e4a75e0ef2d7e83b390f68e833cfa46c164cd0b4542af10a0337e2R35-R36) ("If null occurs in left, if null count in right is not 0, it returns true, else returns null.").

But what I don't understand is that this still seems to be tested that way for the IsIn function:

arrow/cpp/src/arrow/compute/kernels/scalar_set_lookup_test.cc

Lines 107 to 111 in 8b9f6b9

	// Nulls in left array
	CheckIsIn<TypeParam, T>(type, {2, 1, 2, 1, 2, 3},
	{false, false, false, false, false, false}, {2, 1, 2, 1, 3}, {},
	{false, false, false, false, false, false},
	{false, false, false, false, false, false});

Correction: the test I linked to was for only nulls in the left array, and so that's also how it works in Python (my example above is about nulls in both left and right array).

So in summary:

• The default behaviour is to return True for nulls if a null is also present in the right array, and otherwise to return null. So the doc comment in api_scalar.h is wrong about returning False when nulls in the right are not considered (skipped or not present).
• In addition, the keyword itself is still ignored (I suppose it was only implemented for IndexIn, not IsIn)

[asfimport]

Antoine Pitrou / @pitrou:
Issue resolved by pull request 9164
#9164