[REVIEW] Introduce parity with pandas for MultiIndex.loc ordering & fix a bug in Groupby with as_index

[galipremsagar]

Description

This PR:

• Adds ordering in MultiIndex.loc when pandas compatibility mode is enabled.
• Fixes a bug in Groupby when as_index is passed and is not being respected when a getitem call is performed on the groupby.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[bdice]

This looks fine. Once this is merged, please get the permalink for this TODO and copy it into some issue about deterministic join/merge. There is at least one existing issue on this topic but I can’t search for it at this moment.

[galipremsagar]

Thanks @bdice ! Made a note of it in #12236

[galipremsagar]

/merge

[wence-]

Pretty sure this can't do the right thing, because the indices you get back from the merge that lives inside _get_valid_indices_by_tuple don't induce the order you want on the result.

For example (with this change applied):

import pandas as pd
import cudf
df = pd.DataFrame({"a": [1, 1, 2, 3], "b": [1, 0, 1, 1], "c": [1, 2, 3, 4]}).set_index(["a", "b"])
cdf = cudf.from_pandas()
# pandas behaviour
In [24]: df.loc[([1, 1], [0, 1]), :]
Out[24]: 
     c
a b   
1 0  2
  1  1

In [25]: df.loc[([1, 1], [1, 0]), :]
Out[25]: 
     c
a b   
1 1  1
  0  2

# (new) cudf behaviour
n [26]: cdf.loc[([1, 1], [0, 1]), :]
Out[26]: 
     c
a b   
1 1  1
  0  2

In [27]: cdf.loc[([1, 1], [1, 0]), :]
Out[27]: 
     c
a b   
1 1  1
  0  2

Note how in the cudf case both return values are now ordered the same.

Do you instead want something like this in _compute_validity_mask ?

diff --git a/python/cudf/cudf/core/multiindex.py b/python/cudf/cudf/core/multiindex.py
index d8a08647f3..0ee741e066 100644
--- a/python/cudf/cudf/core/multiindex.py
+++ b/python/cudf/cudf/core/multiindex.py
@@ -759,6 +759,8 @@ class MultiIndex(Frame, BaseIndex, NotIterable):
             if isinstance(row, slice) and row == slice(None):
                 continue
             lookup[i] = cudf.Series(row)
+        cantor_name = "_" + "_".join(map(str, lookup.columns))
+        lookup[cantor_name] = column.arange(len(lookup))
         frame = cudf.DataFrame(dict(enumerate(index._data.columns)))
         data_table = cudf.concat(
             [
@@ -769,7 +771,9 @@ class MultiIndex(Frame, BaseIndex, NotIterable):
             ],
             axis=1,
         )
-        result = lookup.merge(data_table)["idx"]
+        result = lookup.merge(data_table).sort_values(by=[cantor_name, "idx"])[
+            "idx"
+        ]
         # Avoid computing levels unless the result of the merge is empty,
         # which suggests that a KeyError should be raised.
         if len(result) == 0:

I note in passing for pandas-2 compat, the merge in _compute_validity_mask should be a left join and a KeyError should be raised for any missing keys (if the null_count of the indices is greater than 0).

But I can't really follow all the back-and-forth here, so let's just leave that for now.

[wence-]

Sorry, I had a review pending but didn't hit go in time. I think this change was wrong unfortunately :(

[galipremsagar]

Sorry, I had a review pending but didn't hit go in time. I think this change was wrong unfortunately :(

Apologies for the merge @wence-, I think the solution you pointed out here: #13657 (comment) seems good to me. Could you open a PR with same changes that I can review & approve instead? I'm not on my workstation, or else I would have done it.

[wence-]

#13660