Repository: pandas-dev/pandas
Pull request: ENH: Move corrwith from transformation to reduction kernels in groupby.base
- [x] closes #31270
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Problem statement:
ENH: Move "corrwith" from transformation_kernels to reduction_kernels in groupby.base
#### Code Sample
```python
>>> import pandas as pd
>>> pd.__version__
'1.0.0rc0+183.g4da554f75'
>>> df = pd.DataFrame(
...     {
...         "A": [1, 1, 1, 1],
...         "B": [0, 1, 2, 3]
...     }
... )

>>> df
   A  B
0  1  0
1  1  1
2  1  2
3  1  3

>>> df.groupby("A").corrwith(df)  # Clearly corrwith is a reduction rather than transformation
     B   A
A
1  1.0 NaN
```

On current master (`1.0.0rc0+183.g4da554f75`) we have "corrwith" in the list of ``transformation_kernels`` rather than ``reduction_kernels`` within `groupby.base`.

We probably have to move it and fix any impact it has.

Hints from issue discussion:
take

Commit message: small change in comments (GH31270)

Patch to review:
diff --git a/pandas/core/groupby/base.py b/pandas/core/groupby/base.py
index 700d8d503d086..0c9beac80761e 100644
--- a/pandas/core/groupby/base.py
+++ b/pandas/core/groupby/base.py
@@ -98,6 +98,8 @@ def _gotitem(self, key, ndim, subset=None):
     [
         "all",
         "any",
+        # `corrwith` is a reduction as long as `other` is a Series
+        "corrwith",
         "count",
         "first",
         "idxmax",
@@ -132,7 +134,6 @@ def _gotitem(self, key, ndim, subset=None):
     [
         "backfill",
         "bfill",
-        "corrwith",
         "cumcount",
         "cummax",
         "cummin",
diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py
index 9b07269811d8e..9ea5252b91e13 100644
--- a/pandas/tests/groupby/test_categorical.py
+++ b/pandas/tests/groupby/test_categorical.py
@@ -1262,6 +1262,9 @@ def test_series_groupby_on_2_categoricals_unobserved(
     if reduction_func == "ngroup":
         pytest.skip("ngroup is not truly a reduction")
 
+    if reduction_func == "corrwith":  # GH 32293
+        pytest.xfail("TODO: implemented SeriesGroupBy.corrwith")
+
     df = pd.DataFrame(
         {
             "cat_1": pd.Categorical(list("AABB"), categories=list("ABCD")),
diff --git a/pandas/tests/groupby/test_transform.py b/pandas/tests/groupby/test_transform.py
index 740103eec185a..2295eb2297fa6 100644
--- a/pandas/tests/groupby/test_transform.py
+++ b/pandas/tests/groupby/test_transform.py
@@ -327,9 +327,9 @@ def test_transform_transformation_func(transformation_func):
         }
     )
 
-    if transformation_func in ["pad", "backfill", "tshift", "corrwith", "cumcount"]:
+    if transformation_func in ["pad", "backfill", "tshift", "cumcount"]:
         # These transformation functions are not yet covered in this test
-        pytest.xfail("See GH 31269 and GH 31270")
+        pytest.xfail("See GH 31269")
     elif _is_numpy_dev and transformation_func in ["cummin"]:
         pytest.xfail("https://github.com/pandas-dev/pandas/issues/31992")
     elif transformation_func == "fillna":
@@ -1093,8 +1093,10 @@ def test_transform_agg_by_name(reduction_func, obj):
         pytest.xfail("TODO: g.transform('ngroup') doesn't work")
     if func == "size":  # GH#27469
         pytest.xfail("TODO: g.transform('size') doesn't work")
+    if func == "corrwith" and isinstance(obj, Series):  # GH#32293
+        pytest.xfail("TODO: implement SeriesGroupBy.corrwith")
 
-    args = {"nth": [0], "quantile": [0.5]}.get(func, [])
+    args = {"nth": [0], "quantile": [0.5], "corrwith": [obj]}.get(func, [])
 
     result = g.transform(func, *args)
 


Code review comments:
diff_hunk: @@ -98,6 +98,8 @@ def _gotitem(self, key, ndim, subset=None):
     [
         "all",
         "any",
+        # `corrwith` is a reduction as long as `other` is a Series
path: pandas/core/groupby/base.py
text: @user1:
Hmm maybe not thinking clearly but when is this not a reduction? I don’t quite understand the comment

@author:
When `other` is a `DataFrame` and it has different column names, then it is not strictly a reduction, as it produces more columns than `self` originally has.

Example:
```python
>>> ser = pd.Series([1,2,3,1,2,3], name='test')
>>> df1 = pd.DataFrame(dict(key=list('aaabbb'), value1=[1,2,3,3,2,1], value2=[1,3,2,2,3,1]))
>>> df2 = pd.DataFrame(dict(col1=[1,2,3,3,2,1], col2=[1,3,2,2,3,1]))
>>> df1
  key  value1  value2
0   a       1       1
1   a       2       3
2   a       3       2
3   b       3       2
4   b       2       3
5   b       1       1
>>> df2
   col1  col2
0     1     1
1     2     3
2     3     2
3     3     2
4     2     3
5     1     1

>>> ser
0    1
1    2
2    3
3    1
4    2
5    3
Name: test, dtype: int64
```

```python
# groupby.corrwith is a reduction when `other` is a Series (the name "test" didn't matter)
>>> df1.groupby('key').corrwith(ser)
     value1  value2
key
a       1.0     0.5
b      -1.0    -0.5

# groupby.corrwith is a reduction when `other` is a DataFrame and has different column names
>>> df1.groupby('key').corrwith(df2)
     col1  col2  value1  value2
key
a     NaN   NaN     NaN     NaN
b     NaN   NaN     NaN     NaN

# not a reduction even if `other` only has 1 column (and that column has a different name),
>>> df1[['key', 'value1']].groupby('key').corrwith(df2[['col1']])
     col1  value1
key
a     NaN     NaN
b     NaN     NaN

# when they have the same column names, it is a reduction
>>> df1.groupby('key').corrwith(df1)
     value1  value2
key
a       1.0     1.0
b       1.0     1.0
```

Initially, I thought I'd put something like this 
```
`corrwith` is a reduction as long as `other` is a `Series` or is a `DataFrame` with the same numerical columns
```
However, the above is a bit confusing and not entirely precise. In fact, it is pretty hard to precisely describe when `corrwith` is a reduction without confusion. See below
```python
# the order of columns, and non-numeric columns ("key") didn't matter
>>> df1.groupby('key').corrwith(df1[['value2', 'value1']])
     value1  value2
key
a       1.0     1.0
b       1.0     1.0

# still a reduction even though `other` has less numerical columns
>>> df1.groupby('key').corrwith(df1[['value2']])
     value2  value1
key
a       1.0     NaN
b       1.0     NaN

# then there is the complication of drop=True
>>> df1.groupby('key').corrwith(df1[['value2']], drop=True)
     value2
key
a       1.0
b       1.0
```

Perhaps we can think of something like this?
```
`corrwith` is a reduction as long as `other` is a `Series` or is a `DataFrame` whose numerical columns are a subset of the numerical columns of `self`
```

@user1:
Hmm OK. I get what you are saying but I don't think the comment really adds value or expresses that. Can you remove?

@author:
ok. done