BUG/PERF: Series.combine passes Series on duplicate index labels

[rhshadrach]

Generated by Claude Fable 5. Reviewed and refined by me.

When Series.combine gets an index with duplicate labels, it starts passing Series to the UDF.

a = pd.Series([1, 2, 3], index=["a", "a", "b"])
b = pd.Series([10, 20], index=["a", "b"])

def func(x, y):
    print(f'x={type(x)}, y={type(y)}')
    return x + y

a.combine(b, func)
# x=<class 'pandas.Series'>, y=<class 'numpy.int64'>
# x=<class 'pandas.Series'>, y=<class 'numpy.int64'>
# x=<class 'numpy.int64'>, y=<class 'numpy.int64'>

The docstring for func states:

Function that takes two scalars as inputs and returns an element.

By moving to this implementation, we also have significant perf improvements.

┌─────────────────────────────┬────────┬────────┐
│            case             │  main  │ branch │
├─────────────────────────────┼────────┼────────┤
│ n=3                         │ 100 µs │ 113 µs │
├─────────────────────────────┼────────┼────────┤
│ n=100                       │ 561 µs │ 242 µs │
├─────────────────────────────┼────────┼────────┤
│ 100k half-overlap           │ 392 ms │ 45 ms  │
├─────────────────────────────┼────────┼────────┤
│ 100k identical index        │ 177 ms │ 26 ms  │
├─────────────────────────────┼────────┼────────┤
│ 100k with one duplicate     │ 3.2 s  │ 76 ms  │
├─────────────────────────────┼────────┼────────┤
│ 100k, heavy dups both sides │ 33.3 s │ 77 ms  │
└─────────────────────────────┴────────┴────────┘

[jbrockmendel]

Does this change any of these cases and are the changes intentional?

left = pd.Series([1, 2], index=pd.date_range("2020", periods=2))
right = pd.Series([10, 20], index=pd.date_range("2020", periods=2, tz="UTC"))
left.combine(right, lambda x, y: x + y, fill_value=0)

mi1 = pd.MultiIndex.from_tuples([("a", 1), ("b", 2)], names=["x", "y"])
mi2 = pd.MultiIndex.from_tuples([("a", 1), ("c", 3)], names=["p", "q"])
left = pd.Series([1, 2], index=mi1)
right = pd.Series([10, 20], index=mi2)
left.combine(right, lambda x, y: x + y, fill_value=0)

left = pd.Series([1, 2], index=mi1)
right = pd.Series([10, 20], index=["a", "b"])
left.combine(right, lambda x, y: x + y)

[rhshadrach]

Thanks @jbrockmendel. Series.combine has always followed union rather than join semantics, so I think the fact that each of your examples raise can be considered a bug in this PR as it was implemented. I believe the union semantics also dictate how duplicates should be treated: pairwise and not cartesian.

If I'm missing something and we feel the duplicate case is ambiguous, we could instead raise. That could technically be breaking behavior. No one has complained about duplicate behavior on the issue tracker at all and I'm wondering if that's due in part to this method not seeing much use. But there could be someone out there using today's dups behavior as-is. While implementing duplicate-handling behavior is 100% defensible as a bugfix here, raising a ValueError is still defensible but I think less so.

Benchmarks in OP have been updated.

[jbrockmendel]

Test class for this?

[jbrockmendel]

was this deleted on purpose?

[jbrockmendel]

is it obvious that this should be [11, 2, 23] and not [11, 12, 23]?

[rhshadrach]

This is the join-vs-union behavior I alluded to in #67446 (comment). From what I can tell, the history has always intended it to be a union here, and combine certainly doesn't follow pandas join-semantics in any cases. That makes it obvious to me it should be [11, 2, 23], but I can't say I feel 100% confident that maybe some of this wasn't purposefully decided.

Assuming we're still good with union-behavior, I'll add a comment here.

[jbrockmendel]

ok, thanks for taking a look