lib/datautils: groupby_agg on string column with missing values introduces 0

[jorisvandenbossche]

While running the tests with the next version of pandas, I ran into this case:

etl/lib/datautils/tests/test_dataframes.py

Lines 309 to 329 in f9e2f93

	def test_default_aggregate_with_some_nans_ignored_different_types_and_more_nans(
	self,
	):
	df_in = pd.DataFrame(
	{
	"year": [2001, 2002, 2002, 2003, 2003, 2003],
	"value_01": [np.nan, 2, np.nan, 4, 5, 6],
	"value_02": [np.nan, "b", np.nan, "d", "e", "f"],
	"value_03": [np.nan, False, False, True, True, np.nan],
	}
	)
	df_out = pd.DataFrame(
	{
	"year": [2001, 2002, 2003],
	"value_01": [0.0, 2.0, 15.0],
	"value_02": [0, "b", "def"],
	"value_03": [0, 0, 2],
	}
	).set_index("year")
	df_out["value_03"] = df_out["value_03"].astype(object)
	assert dataframes.groupby_agg(

where the expected result df_out column "value_02" is a mixed integer/string column, because of a zero being introduced through the groupby's sum on a group with only missing values.

While being tested as the expected result, I am wondering if this is the intended or desired behaviour to get a 0 in there (I assume this is not the case). Or whether this is just from using some dummy data in the test, and not something you encounter or want on the actual datasets? (for example, I don't know if you actually ever use the "sum" operation on a string column in practice)

The reason I ran into this case is because this test is failing with pandas' future string dtype enabled. It is failing because the result now no longer has an integer 0 but a string "0" (and hence the assert equals(..) failed), which I think is even worse (so I opened an upstream issue to fix this in pandas: pandas-dev/pandas#60229, thanks to running your test suite!)

[larsyencken]

@pabloarosado will take a quick look, it may not be trivial for us to make a change to behaviour since it affects a bunch of our steps.

[Marigold]

It took me a while to understand what’s going on. Fortunately, this is dummy data and isn’t very representative of our typical use case. In fact, I’d say running sum on strings would generally be considered bad practice for us.

I’m closing this for now. If it gets fixed upstream, we’ll address it on our side. Thanks, Joris!