ARROW-14656: [Python] sort helper for Array, ChunkedArray and StructArray

[amol-]

No description provided.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-14656

[jorisvandenbossche]

It seems a bit strange to have this method only on StructArray IMO. Also for other arrays, sorting takes some effort with the current API.

[pitrou]

This is sorting by a given child field. So it wouldn't make sense on other arrays (including unions).

[jorisvandenbossche]

But it does make sense to sort just the array.

What I mean is, also for sorting an array, it takes some code:

arr.take(pc.array_sort_indices(arr, order))

vs for struct array:

arr.take(pc.array_sort_indices(arr.field(name), order)

Once you have the first snippet, I don't see why the second snippet would necessarily warrant a helper function (and the first not)

[pitrou]

Well, we can expose both if we want, though @amol- 's ticket seems to be specifically for sort_by :-)

[jorisvandenbossche]

I know, but so my feedback is that I am personally -1 on adding a special case method sort_by

[amol-]

I think it would be resonable to rename the method to StructArray.sort and add it to all Arrays. The signature would be different between them (as in StructArray you have to provide the field argument) but apart from that we can provide a sort method for all arrays and it would have value as it would still prevent combining multiple compute functions to achieve the wanted result.

[amol-]

This is currently blocked by:

• array_sort_indices not working on ChunkedArray: https://issues.apache.org/jira/browse/ARROW-12042
• array_sort_indices not implemented for StructArray

[pitrou]

@amol- Why don't you just sort_indices instead?

[amol-]

@amol- Why don't you just sort_indices instead?

That's what I'm ending up doing for ChunkedArray, I tried passing None as the column name but it explicitly requires a string, so I'll just end up passing an empty one

[jorisvandenbossche]

That's what I'm ending up doing for ChunkedArray, I tried passing None as the column name but it explicitly requires a string, so I'll just end up passing an empty one

Yes, using "dummy" name is also what we do elsewhere:

arrow/python/pyarrow/compute.py

Lines 722 to 723 in 92ee295

	if isinstance(values, (pa.Array, pa.ChunkedArray)):
	sort_keys.append(("dummy", "descending"))

[amol-]

This should be revert to omit the fieldname, it was the acceeptance test for sorting struct arrays wiithout any specific field.

[amol-]

This should be revert to omit the fieldname, it was the acceeptance test for sorting struct arrays wiithout any specific field.

[pitrou]

@amol- What is the status on this? Do you want to take it up?

[amol-]

@amol- What is the status on this? Do you want to take it up?

I think I can take it up if @Jedi18 can't work on it anymore. Or even better would @AlvinJ15 be able to tackle it? He can probably do it faster than me.

[Jedi18]

Yes I'll probably not get the time to take this on at least till June, so please feel free to assign it to someone else. Sorry for holding up this PR for so long!

[jorisvandenbossche]

@amol- I think this PR can be closed?

The python API additions have been merged in #14781, and the C++ additions are also included in the open PR #14369

[amol-]

@amol- I think this PR can be closed?

The python API additions have been merged in #14781, and the C++ additions are also included in the open PR #14369

I think so, I think it was still open only to take the pieces and move them into the other PRs