REGR: Bug fix for ExtensionArray groupby aggregation on non-numeric types

[BryanCutler]

• closes REGR: ExtensionArray aggregation on non-numeric types fails #38980
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

[BryanCutler]

Exception handling that checks for this particular message in order to fallback to non-cython impl is here https://github.com/pandas-dev/pandas/blob/master/pandas/core/groupby/groupby.py#L1047

I can work on adding a unit test for this

[jbrockmendel]

need tests

[jorisvandenbossche]

@BryanCutler this is a small example that I was testing that worked before and is failing now, so you can use that as a base for writing tests if you like

In [1]: df = pd.DataFrame({'key': ['a', 'a', 'b', 'b'], 'val': ['a', 'b', 'c', 'd']}, dtype="string")

In [2]: df.groupby("key").agg({'val': 'first'})  # or 'min'
Out[2]: 
    val
key    
a     a
b     c

Specifying 'sum' doesn't work in case of the "string" dtype, since it raises an error (because for now we don't allow "sum" on a string type, something we probably should). But "sum" apparently takes a slightly different route and doesn't try the cython_agg.

Also with the decimal test extension type that is included in our tests for such testing purposes, it can be reproduced:

from pandas.tests.extension.decimal import DecimalArray, make_data
df = pd.DataFrame({'key': ['a', 'a', 'b', 'b'], 'val': DecimalArray(make_data()[:4])})
df.groupby("key").agg({'val': 'min'})

[jreback]

yep pls add tests

[BryanCutler]

Thanks for the code samples @jorisvandenbossche. I tried to add a base extension test to cover more types and looks good for all except decimal and json, which give the following error:

E           AssertionError: ExtensionArray are different
E           
E           Attribute "dtype" are different
E           [left]:  object
E           [right]: decimal

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=51924&view=logs&j=2d7fb38a-2053-50f3-a67c-09f6e91d3121&t=449937cc-3d50-56b5-5662-e489f41f1268&l=177

Not sure if it's a problem with wrapping back up the extension array, or maybe these tests need a specialization. I'll look into it a little later..

[jreback]

looks like failing on 32-bit

[jorisvandenbossche]

Thanks! Two small suggestions

[jorisvandenbossche]

Suggested change

	result = df.groupby("A").agg({"B": "first"}).B.array
	expected = df["B"].iloc[[0, 2, 4, 7]].array
	self.assert_extension_array_equal(result, expected)
	expected = df["B"].iloc[[0, 2, 4, 7]].array
	result = df.groupby("A").agg({"B": "first"}).B.array
	self.assert_extension_array_equal(result, expected)
	result = df.groupby("A").agg("first").B.array
	self.assert_extension_array_equal(result, expected)
	result = df.groupby("A").first().B.array
	self.assert_extension_array_equal(result, expected)

Those different ways apparently take a somewhat different code path, given that the two added ones actually still worked (but so worth testing those different ways)

[BryanCutler]

@jreback I addressed the feeback and tests have passed. Please take another look when you can, thanks!

[jreback]

thanks @BryanCutler very nice!

[jreback]

@meeseeksdev backport 1.2.x

[jorisvandenbossche]

Thanks @BryanCutler !

[BryanCutler]

Thank you all for reviewing!