BUG: Groupby agg on decimal,json extension arrays changes dtype to object

[BryanCutler]

• I have checked that this issue has not already been reported.

• I have confirmed this bug exists on the latest version of pandas.

• (optional) I have confirmed this bug exists on the master branch of pandas.

---

Code Sample, a copy-pastable example

import decimal
from pandas.tests.extension.decimal import DecimalArray
import pandas as pd

b = decimal.Decimal("1.0")
a = decimal.Decimal("0.0")
c = decimal.Decimal("2.0")
na = decimal.Decimal("NaN")

df = pd.DataFrame({
    'key': [1, 1, 2, 2, 3, 3, 1, 4], 
    'val': DecimalArray([b, b, na, na, a, a, b, c])
})

result = df.groupby("key").agg({'val': 'first'})

print(result['val').dtype)
# object

Problem description

When performing a groupby().agg() on extension arrays DecimalArray or JsonArray, if a NULL value is present, the resulting aggregation will be converted to an object dtype.

Expected Output

Resulting series should be the same dtype as the input array, and any NULL values should be as defined in the extension dtype.

Output of pd.show_versions()

INSTALLED VERSIONS

commit : 1c296b2
python : 3.8.6.final.0
python-bits : 64
OS : Linux
OS-release : 5.4.0-58-generic
Version : #64-Ubuntu SMP Wed Dec 9 08:16:25 UTC 2020
machine : x86_64
processor : x86_64
byteorder : little
LC_ALL : C.UTF-8
LANG : C.UTF-8
LOCALE : en_US.UTF-8

pandas : 1.3.0.dev0+357.g1c296b20f.dirty
numpy : 1.19.4
pytz : 2020.4
dateutil : 2.8.1
pip : 20.0.2
setuptools : 49.6.0.post20201009
Cython : 0.29.21
pytest : 6.1.2
hypothesis : 5.41.5
sphinx : 3.3.1
blosc : None
feather : None
xlsxwriter : 1.3.7
lxml.etree : 4.6.2
html5lib : 1.1
pymysql : None
psycopg2 : None
jinja2 : 2.11.2
IPython : 7.19.0
pandas_datareader: None
bs4 : 4.9.3
bottleneck : 1.3.2
fsspec : 0.8.4
fastparquet : 0.4.1
gcsfs : 0.7.1
matplotlib : 3.3.3
numexpr : 2.7.1
odfpy : None
openpyxl : 3.0.5
pandas_gbq : None
pyarrow : 2.0.0
pyxlsb : None
s3fs : 0.4.2
scipy : 1.5.3
sqlalchemy : 1.3.20
tables : 3.6.1
tabulate : 0.8.7
xarray : 0.16.2
xlrd : 1.2.0
xlwt : 1.3.0
numba : 0.52.0

[BryanCutler]

I was debugging this while working on #38982, so I'll give it a shot to fix

[jbrockmendel]

@BryanCutler this looks fixed on master, was #38982 the one to fix it?

[BryanCutler]

@jbrockmendel it looks like it was fixed for DecimalArray as part of #39381 , specifically https://github.com/pandas-dev/pandas/pull/39381/files#diff-c87a2542fa72eab7f417009d906f4b27ae30585a49b7097cc4787a835e95b5deR56.

The problem still exists for JSONArray or any extension type that doesn't check for np.nan in the constructor. I think the correct fix is for groupby agg to use ExtensionDtype.nan_value instead of np.nan in the results, does that sound reasonable?

[jbrockmendel]

I think the correct fix is for groupby agg to use ExtensionDtype.nan_value instead of np.nan in the results, does that sound reasonable?

yes

[BryanCutler]

Ok sounds good. I'll try to submit a PR with the above fix soon. Thanks!

[MichaelTiemannOSC]

This seems to actually work in the latest version of Pandas, but it's marked XFAIL in pandas/tests/extension/json/test_json.py::TestGroupby::test_groupby_agg_extension so the success actually messes up the test result.

(Pdb) print(result['val').dtype)
*** SyntaxError: closing parenthesis ')' does not match opening parenthesis '['
(Pdb) print(result['val'].dtype)
decimal

Note that copying the above code results in the syntax error, but when fixed, the result has the correct EA Dtype (which is decimal, not object).

Output of pd.show_versions():

(Pdb) pd.show_versions()

INSTALLED VERSIONS

commit : a165278
python : 3.9.16.final.0
python-bits : 64
OS : Darwin
OS-release : 22.5.0
Version : Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64
machine : x86_64
processor : i386
byteorder : little
LC_ALL : None
LANG : en_US.UTF-8
LOCALE : en_US.UTF-8

pandas : 2.1.0.dev0+1122.ga165278a61.dirty
numpy : 1.24.3
pytz : 2023.3
dateutil : 2.8.2
setuptools : 67.7.2
pip : 23.1.2
Cython : 0.29.35
pytest : 7.3.2
hypothesis : 6.75.1
sphinx : 5.1.1
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 3.1.2
IPython : 8.13.1
pandas_datareader: None
bs4 : 4.12.2
bottleneck : None
brotli :
fastparquet : None
fsspec : 2023.4.0
gcsfs : None
matplotlib : 3.7.1
numba : None
numexpr : None
odfpy : None
openpyxl : 3.1.2
pandas_gbq : None
pyarrow : 8.0.0
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : None
snappy : None
sqlalchemy : 1.4.46
tables : None
tabulate : None
xarray : None
xlrd : 2.0.1
zstandard : 0.19.0
tzdata : 2023.3
qtpy : 2.3.1
pyqt5 : None

[jbrockmendel]

i get that there is a typo in the OP, but I dont understand what you're saying is wrong with the xfailed test. Are you saying it should not be xfailed and somehow the CI isn't catching that?

[MichaelTiemannOSC]

Yes. I believe that the xfail is obsolete, as proved by the correct output (after fixing the typo).

Question: if I remove the XFAIL in a PR that I'm working on, will CI/CD work against my test code, or does it check its own test code independently? I'm very new to Pandas (but wrote GNU C++ back in the day, so do have some dev experience, if very old).

[jbrockmendel]

All the tests should be run for every PR, including xfailed ones. If an xfailed test passes the CI should alert us to that.

[MichaelTiemannOSC]

Let me rephrase my question. Should I put this in my PR, put it into a new issue, or just leave it be and wait for the CI world to update the test suite?

(itr_env) iMac-Pro:pandas-dev michael$ git diff --minimal
diff --git a/pandas/tests/extension/json/test_json.py b/pandas/tests/extension/json/test_json.py
index 0920e70142..b2b0278d6f 100644
--- a/pandas/tests/extension/json/test_json.py
+++ b/pandas/tests/extension/json/test_json.py
@@ -356,7 +356,9 @@ class TestGroupby(BaseJSON, base.BaseGroupbyTests):
         """
         super().test_groupby_extension_no_sort()
 
-    @pytest.mark.xfail(reason="GH#39098: Converts agg result to object")
+    @unhashable
+    # GH#39098 is now fixed, so we don't need to XFAIL
+    # @pytest.mark.xfail(reason="GH#39098: Converts agg result to object")
     def test_groupby_agg_extension(self, data_for_grouping):
         super().test_groupby_agg_extension(data_for_grouping)

[jbrockmendel]

put it in your PR. worst case scenario is someone asks you to split it out into a separate PR. Best case scenario my confusion gets resolved