TST: add test case for user-defined function taking correct path in groupby transform

[jbrockmendel]

I kept this one in place because I thought fast_path could be user-defined (see comment on L1425), but it turns out we define it in _define_paths a few lines above. So this try/except is unnecessary.

[jorisvandenbossche]

it turns out we define it in _define_paths a few lines above

But the func in _define_paths can still be a UDF?

[jreback]

@jbrockmendel these are all failing for the web / docs stuff?

[jbrockmendel]

But the func in _define_paths can still be a UDF?

Yes, but the sole call to _choose_path is itself wrapped in a try/except TypeError.

It's entirely possible that I'm missing something. In recent threads you've shown a talent for finding corner cases that are currently missing from the tests. Any ideas for what cases will raise here that we should worry about?

[jbrockmendel]

these are all failing for the web / docs stuff?

Not sure yet whats going on there. Could well be an example we can/should add to the tests!

[jorisvandenbossche]

Not sure yet whats going on there. Could well be an example we can/should add to the tests!

Probably yes, it's in any case a transform with a UDF that is failing

[jbrockmendel]

@jorisvandenbossche i found the case from cookbook.rst that is failing in this PR, but it isn't clear to me that it is behaving correctly on master either. I'd like to get a second opinion.

df = pd.DataFrame({'A': [1, 1, 2, 2], 'B': [1, -1, 1, 2]})
gb = df.groupby('A')

def replace(g):
    mask = g < 0
    return g.where(mask, g[~mask].mean())

result = gb.transform(replace)

>>> result
     B
0  1.0
1 -1.0
2  1.5
3  1.5

I expected result.loc[1] to match result.loc[0]. Can you double-check me on this?

[jorisvandenbossche]

It's what the function seems to be doing:

In [22]: group1 = gb.get_group(1)

In [23]: replace(group1['B']) 
Out[23]: 
0    1
1   -1
Name: B, dtype: int64

So maybe the function is doing something unexpected, but it seems the transform is correctly using the result of the function on the group.

I find where always a difficult one to reason about, but basically this where call is keeping the original negative values, and replacing the positive values with the mean of those positive values. So in this case, that result seems correct to me (as the mean of the positive values is also 1).

[jbrockmendel]

@jorisvandenbossche for the immediate purpose of this PR, I'm implementing a test case where fast_path raises, but I don't want to use a not-clearly-correct case. Can you suggest one? (this might be obvious, but i need caffeine)

The follow-on issues are a) making sure the example in the docs is correct and b) making sure where is correct.

[jorisvandenbossche]

As far as I understand, the example in the docs is correct

[jbrockmendel]

Updated. Now the behavior is unchanged, just added a test that requires the current behavior. Thanks for helping me understanding it @jorisvandenbossche

[jorisvandenbossche]

I think it might be good to use a simpler function to test here (otherwise somebody later looking at the test will have the same "what the heck is this UDF doing" thought as we had now). It just needs to be a function that accepts a Series / returns a Series of the same shape, I think

[jbrockmendel]

updated

[jorisvandenbossche]

Thanks, looks good!

[jreback]

thanks @jbrockmendel