[C++][Python] Allow pyarrow.compute.mode to include null count

[mroeschke]

Describe the enhancement requested

There is a skip_nulls argument to dictate whether nulls should make the result null or be skipped, but it would be potentially be useful for mode to return whether null was a mode of the dataset

Component(s)

Python

[jorisvandenbossche]

I have to say that the current skip_null keyword is a bit strange, and it also doesn't do what is documented:

>>> pc.mode([1, 2, 2, None, None], 2, skip_nulls=True) 
<pyarrow.lib.StructArray object at 0x7f64ea3ab940>
-- is_valid: all not null
-- child 0 type: int64
  [
    2,
    1
  ]
-- child 1 type: int64
  [
    2,
    1
  ]

>>> pc.mode([1, 2, 2, None, None], 2, skip_nulls=False)
<pyarrow.lib.StructArray object at 0x7f64ea4c3e20>
-- is_valid: all not null
-- child 0 type: int64
  []
-- child 1 type: int64
  []

So if there are nulls and you specify to not skip them, the result becomes empty, while the docs say that if any input value is null, the output becomes null for skip_nulls=False (but null and empty is not the same).

I am not fully sure what "mode" should do with nulls. On the one hand, the mode kernel is related to "value_counts", and this kernel will also count nulls. On the other hand, it's more seen as a (numerical) aggregation kernel, and those all skip (by default) or just propagate nulls (I don't think there is another aggregation kernel that really "does" something with the nulls).

cc @cyb70289

[jorisvandenbossche]

Some previous related discussion (but about NaNs, not null): https://issues.apache.org/jira/browse/ARROW-9723

And it seems that the mode kernel initially emitted null for skip_nulls=False, but this was changed to an empty struct in #8637 (the PR that introduced the ability to return the n most occurring values and their count).
So in any case the documentation is not fully up to date with this change in behaviour.

[cyb70289]

I must admit it's super hard for me to catch the meaning of skip_nulls=False. Is this "triple negation"? 😂

[westonpace]

From an SQL perspective "null" means "this value is unknown" which is why skip_nulls=False yields null on something like sum. 1 + 2 + 3 + "some unknown number" yields "something unknown".

Mode is pretty weird where you might know the correct answer even if you don't know all the numbers. So I think the correct behavior of skip_nulls=False should be (apologies for the bad pseudo-code):

if (the count of the second most frequent element + the null count < the count of the most frequent element):
  return the most frequent element
else:
  return null

Therefore, if null is the most frequent element, then I think null should be the result when skip_nulls is false.

If skip_nulls=True then the behavior should be the same as mode(x.filter(pc.is_valid(x)) and should only return null if every element is null.

[jorisvandenbossche]

So I think the correct behavior of skip_nulls=False should be (apologies for the bad pseudo-code):

The problem with this logic is that the mode kernel also returns the count (since we support returning more than just the "top" mode in #8637). And so what value would you use for the count if you take into account the null count like that in case there is an actual most frequent element?

If skip_nulls=True then the behavior should be the same as mode(x.filter(pc.is_valid(x)) and should only return null if every element is null.

Agreed that this is the typical logic, but so currently for an empty array (or all-null array after skipping the nulls), we return an empty result, not null:

>>> pc.mode(pa.array([], pa.int64()), 1, skip_nulls=True) 
<pyarrow.lib.StructArray object at 0x7fbdaf658580>
-- is_valid: all not null
-- child 0 type: int64
  []
-- child 1 type: int64
  []

Should that be changed to a null struct / struct of nulls?

[westonpace]

Ah, I see the problem now. If we are going to be pedantic about it then "an aggregate function returns exactly one row" and the return type should be ListType(list<item: struct<mode: int64, count: int64>>). In the presence of nulls the answer is then an array of type ListType(list<item: struct<mode: int64, count: int64>>) with exactly one null element.

Now, it seems that mode is properly registered as a vector function and not an aggregate function. So we don't have to be pedantic (e.g. you can't use mode in Acero anyways so why bother trying to follow the rules). Given this, I would say that we should return whatever is most convenient. I think that would probably be to treat null as its own element type and return:

pc.mode([1, 2, 2, None, None], 2, skip_nulls=False)
<pyarrow.lib.StructArray object at 0x7f5aa7b5cd00>
-- is_valid: all not null
-- child 0 type: int64
  [
    2,
    null
  ]
-- child 1 type: int64
  [
    2,
    2
  ]

[jorisvandenbossche]

I think that would probably be to treat null as its own element type and return:

So to be explicit: count nulls as separate values like value_counts does, and thus essentially do what the OP asked?

[westonpace]

So to be explicit: count nulls as separate values like value_counts does, and thus essentially do what the OP asked?

Yes.

In the future it might be nice to have a variation on mode which returns a single row. I suppose the big challenge with mode is that it isn't decomposable. You can't break it up into a "merge" and "consume" step.

However, these non-decomposable aggregates are often good fits for window functions. Or for a "group by" where you have many small groups. Or, once spilling is implemented, there are ways you can tackle it with larger groups too. So it would be good to have a new arrow::compute::Function::Kind someday. Perhaps WHOLE_AGGREGATE or WINDOW_AGGREGATE or something. The rules would be:

• Must output a single value (one row one column, e.g. scalar)
• Can expect to receive all of the data as one large table

I can open up a new issue for this.