perf(rust, python): Faster is_sorted when no flag set

[magarick]

This will check sortedness without sorting the entire series even when no flag is set. Seems faster.

[ritchie46]

Nice TODO hunt. :)

[s-banach]

Does this break is_sorted on struct columns?

[magarick]

Does this break is_sorted on struct columns?

Not that I know of. It seems fine. Did you see something?

[s-banach]

In polars = 0.18.8, I get the following, maybe or maybe not related to this PR:

import polars as pl
df = pl.DataFrame({"x": [1, 2], "y": [3, 4]})
df.select(pl.struct("x", "y")).to_series().is_sorted()

PanicException: not implemented

[magarick]

You're right. I tried with 0.18.7
I think I know what the issue is.

[magarick]

Inequality operations aren't implemented for structs. It makes sense to have them. After all, if you can sort structs, you can compare them. But some care is required.
It seems like I could either

1. Implement it at the ChunkedArray level, but this is a bad idea because it requires checking the fields and possibly returning an error if the structs aren't the same type internally. That would require changing return types and introduces higher level behavior than feels appropriate there.
2. Implement it at the Series level. Those functions already return a PolarsResult and a little extra checking for structs, then building up an output seems fine.
3. Special case it for is_sorted and check each field individually if the Series is a Struct.

I'm inclined toward option 2 but I'll defer to @ritchie46 since he knows the internals much better.

[ritchie46]

We should do this in two steps. First fix the regression by pattern matching on structs (and maybe lists) as well. For those we can restore old behavior.

Next we can see if we can implement comparison on structs. This needs to be implemented on the StructChunked struct. A quick way to implement this is doing field wise comparison and then combine the boolean outputs with an AND operation. Later we can check if we need to optimize it further.

[s-banach]

I thought structs were sorted in dictionary order,
eg (0, 1) < (1, 0),
so I don’t understand what you mean by combining the fieldwise comparisons with AND.

[ritchie46]

I thought structs were sorted in dictionary order, eg (0, 1) < (1, 0), so I don’t understand what you mean by combining the fieldwise comparisons with AND.

I am talking about equality inly in this case. The other one needs row encoding. Probably that's the proper wat for all struct comparisons.

[magarick]

Equality is already implemented for Structs, the issue is inequality checks, which aren't implemented in ChunkCompare. The only issue is defining the output when schemata don't match. You might have to change the associated type from BooleanChunked to PolarsResult<BooleanChunked> because non-comparable structs could either give
a. All false (probably not what you want)
b. All null (maybe what you want, but then you're conflating this case with comparing against a null value)
c. Raise an error

Cases:

1. Names and types are equal, but ordering is different. Probably don't want to mess with reordering.
2. Names are different but field types are all the same and in the same order. Maybe this is ok to compare, maybe not.
3. Names are the same, types are the same. Do the comparison!
4. Anything else: return all nulls or fail

Honestly, I think I could do the proper change in not much longer than special-casing a fix, but if you'd like time to think about and discuss how to handle the operation properly we can do it in two phases.