REF: Decouple Series.apply from Series.agg
State: closed

This PR makes `Series.apply` not call `Series.agg`, when given a list-like or dict-like, decoupling the two methods (partly) and solves https://github.com/pandas-dev/pandas/pull/53325#discussion_r1201397825. This makes the code base clearer by making `Series.apply` and `Series.agg` going less back and forth between each other, which was very confusing. Merging this PR paves the way for merging #53325 afterwards.

To decouple this, I've added a new parameter `by_row` to `Series.apply`. It defaults to `True`, keeping current behavior for backward compatibility when given a single callable and internally calling `apply` with `by_row=False`, when given a list-like or dict-like, also for backward compatibility (needed when doing e.g. `ser.apply([lambda x: x])`.

This parameter is also relevant for #52140, where I proposed adding the parameter to solve a different set of problems with `Series.apply`. If this PR gets accepted, the solution to #52140 will be to change `by_row` from `True` to `False` after a deprecation process (plus removing the parameter altogether even longer term).

Also, I've renamed `apply_multiple` to `apply_list_or_dict_like`, as I think that name is clearer.

EDIT: Originally I called new parameter `array_ops_only`, but I've changed it to `by_row`

Labels:
name: Refactor
description: Internal refactoring of code
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
Certainly agreed with where this is going, but I'm not sure this is the right way to get there.

After the deprecation of the default value, the plan is to then deprecate this argument in its entirety, right? So all uses of this method would need to be changed twice

> Merging this PR paves the way for merging https://github.com/pandas-dev/pandas/pull/53325 afterwards.

That can only be merged after the default value of `array_ops_only` is changed to `True` and the argument itself is removed, is that right? Otherwise `agg` can still be taking the `apply` path. So in order to enforce the deprecation in #53325, we'll have to wait for 3 major releases.

The following is dependent on me being correct above, and if I'm not then it can probably be ignored.

The change here and #53325 by itself wouldn't be concerning, but we've been talking about making many changes to the apply/agg/transform API, some of which hinges on the issues here being fixed. It seems complex, noisy for users, and slow to do all this piecemeal to me. This is why I prefer something like https://github.com/pandas-dev/pandas/issues/41112. 

I think we should not attempt both routes simultaneously (piecemeal + something like #41112), so we should decide on a route to move forward.
> That can only be merged after the default value of array_ops_only is changed to True and the argument itself is removed, is that right?

It is actually intended to be merged right after this. If you check the code path in this PR for a hypothetical `ser.apply([func])` call, we end up in `Apply._apply_list_like` with `op_name="apply"`, which means that we will (for each func in a list of funcs) no longer call `ser.agg(func)` like in main, but will instead call `ser.apply(func, array_ops_only=True)`. 

This means that calls to `Series.apply` with list and dicts of callables will no longer call `Series.agg` (like single callables already don't call `Series.agg`). So, after this PR, changes to `Series.agg` (like #53325) can no longer affect the behavior of `Series.apply`.

So this PR and #53325 can be implemented as-is, AFICS (though this is obviously quite complex, and could definitely use more eyes on it to verify whether I'm missing something).

Implementing #52140 will however require deprecating `array_ops_only=False` in v2.x and making users set `array_ops_only=True` in their code to be compatible with v3.0. So if users have set `array_ops_only=True` in their last version before upgrading to v3.0, everything will work unchanged in v3.0. The parameter `array_ops_only` will have to exist in v.3.0 to avoid raising when jumping version from 2.x to v3.0, but setting `array_ops_only` in `Series.apply` will have no effect, except emitting a FutureWarning that the parameter will be removed in the future. This empty parameter will have to keep dangling there until v4.0, but that will though not affect any code in v3.x at all.  So if we implement #52140, all code will be in place in v3.0 and the only vestige of the old world will be a non-functioning & deprecated ghost parameter `array_ops_only`in Series.apply. 

Above I just discuss your first half (before "The following is dependent..."). I think we can maybe discuss the later part in a follow-up, as I guess that discussion can depend on the conclusions for the first part. 
Thanks for the correction; I agree with your assessment. Still, I think my concerns from my previous comment remain. In particular

> I think we should not attempt both routes simultaneously (piecemeal + something like #41112), so we should decide on a route to move forward.

Do you agree with this line here?
I may not be experienced enough with the groupby and window methods to know, but I think it's worth looking into if the `Series` and `Dataframe` methods can have their undesired behavior deprecated using a normal process, i.e we can avoid a parallel implementation for them.

TBH, this has always been a very complex area and it was only after doing #53362 ( i.e. very recently) that I have started  thinking it could be possible to do this with a normal deprecation process. I may still have missed something and be proven wrong of course, but that's part of the discussion...

The way I see it that after this PR and #53325 for example `SeriesApply.agg` will be (shortened a bit):

```python
    def agg(self):
        result = super().agg()
        if result is None:
            obj = self.obj
            f = self.f

            try:
                result = obj.apply(f)
            except (ValueError, AttributeError, TypeError):
                result = f(self.obj)
            else:
                msg = (
                    f"using {f} in {type(obj).__name__}.agg cannot aggregate and "
                    f"has been deprecated. Use {type(obj).__name__}.transform to "
                    f"keep behavior unchanged."
                )
                warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
            return result
```

The above just deprecates calling `obj.apply(f)` and we above actually  not guaranteed that `f(obj)` returns a aggregated value, for example `f =lambda x: np.sqrt(x)`, then `agg` will return transformed values, not a aggregate.

But if we change the above to:

```python
    def agg(self):
        result = super().agg()
        if result is None:
            obj = self.obj
            f = self.f

            try:
                result = obj.apply(f)
            except (ValueError, AttributeError, TypeError):
                result = f(self.obj)

        if not self._is_aggregate_value(result):  # aside: how do we know something is an aggregate value?
                msg = (
                    f"using {f} in {type(obj).__name__}.agg cannot aggregate and "
                    f"has been deprecated. Use {type(obj).__name__}.transform to "
                    f"keep behavior unchanged."
                )
                warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())

        return result
```

then returning a non-aggregate value will emit a warning. So the question is if the above is backward compatible and, if the user fix their code to not emit warnings, their code will be compatible with v.3.0.:

In pandas v3.0 the method will become:

```python
    def agg(self):
        result = super().agg()
        if result is None:
            result = self.f(self.obj)

        if not self._is_aggregate_value(result):
                msg = (
                    f"using {f} in {type(obj).__name__}.agg cannot aggregate and "
                    f"has been deprecated. Use {type(obj).__name__}.transform to "
                    f"keep behavior unchanged."
                )
                warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())

        return result
```

The same can be done similarly with `Series.transform` where we also need to check the result is a transformed result and not an aggregation or something else. To do it with `Series.apply` requires a `array_ops_only` parameter however.

My suspicion is that if the above can be done on `Series.(agg|transform|apply)`, then `DataFrame.(agg|transform|apply)` will also fall into place easily. I haven't looked enough into `(Groupby|Resample|Window).(agg|transform|apply)` enough to have a clear opinion, but maybe everything will fall into place like a puzzle there also, if the `Series`/`DataFrame` methods can be deprecated correctly. And if not, the new `Series`/`DataFrame` methods can be used in a new versions of `(Groupby|Resample|Window)` methods, easing their implementations?

> TBH, this has always been a very complex area and it was only after doing #53362 ( i.e. very recently) that I have started thinking it could be possible to do this with a normal deprecation process.

Indeed, you've made progress where I didn't think it was possible.

For agg, I think pandas should not take a stance on what an aggregated value is but rather treat the return from the UDF _as if_ it were a scalar (even when you would typically think it's not). But we can discuss this at a later point.

> I haven't looked enough into (Groupby|Resample|Window).(agg|transform|apply) enough to have a clear opinion, but maybe everything will fall into place like a puzzle there also, if the Series/DataFrame methods can be deprecated correctly.

For UDFs (as opposed to string aliases), these implementations are largely independent of that in Series/DataFrame.

> And if not, the new `Series`/`DataFrame` methods can be used in a new versions of `(Groupby|Resample|Window)` methods, easing their implementations?

Agreed! Let's move forward with these and see where we get to.

I can see it needs tests for dict_likes, do you have anything else in mind?

EDIT: and also `by_row = (True|False)`.
Yep - was really just thinking by_row
I've updated the tests.
I've updated the PR.
Just out of curiosity - are you seeing each of my reviews get duplicated too?

![image](https://github.com/pandas-dev/pandas/assets/45562402/3412c938-4c54-43e1-8bd4-fef0bbd355ab)

> Just out of curiosity - are you seeing each of my reviews get duplicated too?

yes, pretty weird.


Thanks @topper-123