LinkedDataFrame linkage indexes do not update on groupby

[aclarry]

Overview

GGHM demand model code makes extensive use of LinkedDataFrame.groupby, which worked under old versions of pandas, but are broken in pandas >=0.25.

Reproduction:

Install cheval with pandas >=0.25, and run the following

df1_dict = {
    "df2_id": [1,     2,   3,   1,   2,   3],
    "cats":   ["M", "F", "M", "F", "M", "F"],
}

df2_dict = {
    "col1":   ["a", "b", "c"],
}

df1 = LinkedDataFrame(pd.DataFrame(df1_dict))
df2 = LinkedDataFrame(pd.DataFrame(df2_dict))

df1.link_to(df2, "df2", on_self="df2_id")

for cat, group_df in df1.groupby("cats"):
    group_df.df2.col1 # ValueError: Length of values (6) does not match length of index (3)

This is happening because the private method interception to update the linkage indexes (_take and _take_with_is_copy) is no longer getting called in newer versions of pandas.

Recommended fix:

Find a less hack-ey method to update indexes. Either:

• Move to having all updates happen in a stable public endpoint (likely __finalize__)
• Have all linkages check indexes when they are accessed and update them on-the-fly if necessary (would have to be careful, as a naïve approach may subtly break code if the indexes are the same length)

If performance poses a concern, we could do both and use a stable endpoind, but do lazy update of indexes.

Note:

In pandas 1.3, there is a separate bug which causes LinkedDataFrames to not be properly initialized during groupbys: see pandas issue #45363. This would result in an infinite recursion on LinkedDataFrame.__getattr__ when doing basically anything on the grouped LinkedDataFrame objects, and so the above code would issue a RecursionError.

This issue is basically unfixable on our end; it was fixed in pandas 1.4, but we may want cheval to issue a warning if it detects this incompatible version of pandas.

[PeterKucirek]

Chevl's LDF builds on previous prototypes, which used __finalize__ as you've suggested. But this was incredibly slow when using population-sized data. Basically, __finalize__ gives you the original parent frame and the child frame but not the indexer used to get between them. So you double the indexing time for each operation! This would erase all the performance gains in the current implementation of LDF.

On-the-fly is also inefficient, unless you implement caching. But that just kicks the can down the road; in practice there would be no point in implementing an unused linkage.

The best-of-the-best way to handle this is to integrate LDF functionality into Pandas proper, as only they have the power to update their internals consistently. But that would take some time, at least half a year.

I will take your example code here and see if I can find the new internal function that should be super()ed for this version of Pandas.

[PeterKucirek]

Update: I've looked into the call-stack for Pandas 1.4, and it looks like now DataFrame.groupby() uses just the standard take() method. I think that before it was using a private _take() method, which is already implemented in LDF. In theory, adding the following code should work:

    def take(self, indices, axis=0, is_copy=None, **kwargs):
        sliced = super().take(indices, axis=axis, is_copy=is_copy, **kwargs)
        if axis == 0:
            self.__subset_links(sliced, indices)
        return sliced

I'm unable to test this out. For an unknown reason, I'm getting an unrelated error when I try to test this. I say "unrelated" because it persists even when I revert the code.