REGR: 1.3 behavior change with groupby, apply and side effect

[aberres]

Code Sample, a copy-pastable example

def apply_func(df):
    del df["MEMBER_ID"]

    return df.head(1)


df = pd.DataFrame.from_dict(
    {
        "MONTH": {0: "April", 1: "April", 2: "April"},
        "MEMBER_ID": {0: "Member A", 1: "Member A", 2: "Member B"},
        "ACTIVITY_CATEGORY": {
            0: "Activity 1",
            1: "Days off at homebase",
            2: "Activity 1",
        },
        "FTE": {0: 1.0, 1: 1.0, 2: 0.75},
        "FTE_OFF_DAYS": {0: 5, 1: 5, 2: 10},
    }
)

df.groupby(["MONTH", "FTE"], observed=True)[
    ["MEMBER_ID", "FTE_OFF_DAYS", "ACTIVITY_CATEGORY"]
].apply(apply_func)

Problem description

Deleting the row passed to apply_func in combination with the call to head causes an exception in 1.3rc1.

When apply_func is called the second time the MEMBER_ID column does not exist anymore.

Doing a df = df.copy() in apply_func fixes the problem.

Expected Output

While one could argue that the code is a bit bogus (and should be possibly rewritten), I wonder if this change happened on purpose.

[jreback]

mutating groupby is clearly called out in the docs -

[simonjayhawkins]

if you add print(df) to the user func, you get an interesting traceback

  MEMBER_ID  FTE_OFF_DAYS ACTIVITY_CATEGORY
2  Member B            10        Activity 1
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-7-9dfc20552eea> in <module>
      2     ["MEMBER_ID", "FTE_OFF_DAYS", "ACTIVITY_CATEGORY"]
      3 ]
----> 4 result = grp.apply(apply_func)
      5 print(result)

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/groupby/groupby.py in apply(self, func, *args, **kwargs)
   1251         with option_context("mode.chained_assignment", None):
   1252             try:
-> 1253                 result = self._python_apply_general(f, self._selected_obj)
   1254             except TypeError:
   1255                 # gh-20949

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/groupby/groupby.py in _python_apply_general(self, f, data)
   1285             data after applying f
   1286         """
-> 1287         keys, values, mutated = self.grouper.apply(f, data, self.axis)
   1288 
   1289         return self._wrap_applied_output(

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/groupby/ops.py in apply(self, f, data, axis)
    779             try:
    780                 sdata = splitter.sorted_data
--> 781                 result_values, mutated = splitter.fast_apply(f, sdata, group_keys)
    782 
    783             except IndexError:

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/groupby/ops.py in fast_apply(self, f, sdata, names)
   1324         # must return keys::list, values::list, mutated::bool
   1325         starts, ends = lib.generate_slices(self.slabels, self.ngroups)
-> 1326         return libreduction.apply_frame_axis0(sdata, f, names, starts, ends)
   1327 
   1328     def _chop(self, sdata: DataFrame, slice_obj: slice) -> DataFrame:

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/_libs/reduction.pyx in pandas._libs.reduction.apply_frame_axis0()

<ipython-input-6-d39958370ffe> in apply_func(df)
      1 def apply_func(df):
----> 2     print(df)
      3     del df["MEMBER_ID"]
      4 
      5     return df.head(1)

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/frame.py in __repr__(self)
    992         else:
    993             width = None
--> 994         self.to_string(
    995             buf=buf,
    996             max_rows=max_rows,

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/frame.py in to_string(self, buf, columns, col_space, header, index, na_rep, formatters, float_format, sparsify, index_names, justify, max_rows, min_rows, max_cols, show_dimensions, decimal, line_width, max_colwidth, encoding)
   1128                 decimal=decimal,
   1129             )
-> 1130             return fmt.DataFrameRenderer(formatter).to_string(
   1131                 buf=buf,
   1132                 encoding=encoding,

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/format.py in to_string(self, buf, encoding, line_width)
   1051 
   1052         string_formatter = StringFormatter(self.fmt, line_width=line_width)
-> 1053         string = string_formatter.to_string()
   1054         return save_to_buffer(string, buf=buf, encoding=encoding)
   1055 

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/string.py in to_string(self)
     23 
     24     def to_string(self) -> str:
---> 25         text = self._get_string_representation()
     26         if self.fmt.should_show_dimensions:
     27             text = "".join([text, self.fmt.dimensions_info])

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/string.py in _get_string_representation(self)
     38             return self._empty_info_line
     39 
---> 40         strcols = self._get_strcols()
     41 
     42         if self.line_width is None:

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/string.py in _get_strcols(self)
     29 
     30     def _get_strcols(self) -> list[list[str]]:
---> 31         strcols = self.fmt.get_strcols()
     32         if self.fmt.is_truncated:
     33             strcols = self._insert_dot_separators(strcols)

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/format.py in get_strcols(self)
    538         Render a DataFrame to a list of columns (as lists of strings).
    539         """
--> 540         strcols = self._get_strcols_without_index()
    541 
    542         if self.index:

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/format.py in _get_strcols_without_index(self)
    791             str_columns = [[label] for label in self.header]
    792         else:
--> 793             str_columns = self._get_formatted_column_labels(self.tr_frame)
    794 
    795         if self.show_row_idx_names:

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/io/formats/format.py in _get_formatted_column_labels(self, frame)
    870         else:
    871             fmt_columns = columns.format()
--> 872             dtypes = self.frame.dtypes
    873             need_leadsp = dict(zip(fmt_columns, map(is_numeric_dtype, dtypes)))
    874             str_columns = [

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/generic.py in dtypes(self)
   5640         """
   5641         data = self._mgr.get_dtypes()
-> 5642         return self._constructor_sliced(data, index=self._info_axis, dtype=np.object_)
   5643 
   5644     def astype(

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/series.py in __init__(self, data, index, dtype, name, copy, fastpath)
    431                 index = ibase.default_index(len(data))
    432             elif is_list_like(data):
--> 433                 com.require_length_match(data, index)
    434 
    435             # create/copy the manager

~/miniconda3/envs/pandas-1.3.0rc1/lib/python3.9/site-packages/pandas/core/common.py in require_length_match(data, index)
    525     """
    526     if len(data) != len(index):
--> 527         raise ValueError(
    528             "Length of values "
    529             f"({len(data)}) "

ValueError: Length of values (3) does not match length of index (2)

[simonjayhawkins]

mutating groupby is clearly called out in the docs -

for info

first bad commit: [9ee8674] REF: avoid catching all exceptions in libreduction (#38285)

cc @jbrockmendel

[jbrockmendel]

if you add print(df) to the user func, you get an interesting traceback

Yah im pretty sure this is due to the block.values pinning we do in libreduction, which in this case leads to the size of the data in the DataFrame to not match the length of the columns.

If there's an outcry, reverting #38285 wouldn't be the end of the world. But mutating within groupby apply is a constant headache and the only real solution is to not allow it.

[simonjayhawkins]

If there's an outcry, reverting #38285 wouldn't be the end of the world. But mutating within groupby apply is a constant headache and the only real solution is to not allow it.

I'll label as regression and milestone as 1.3 and assume we will revert #38285 for 1.3 and disallow mutation in a future version.

[jreback]

-1 on reverting - this is not supported in any way and noted in the docs

we could have a better error message though

[simonjayhawkins]

we could have a better error message though

sure. if we can do for 1.3 and add something to the api breaking changes of the release notes, otherwise I see no harm in reverting #38285 to keep the api stable for users.

[rhshadrach]

mutating groupby is clearly called out in the docs -

Agreed mutating should not be supported after 1.3, but it is not in the public docs until 1.3 is released. With this in mind, we could revert #38285 for 1.3 and reinstating it for the next release.

That said, I'm +1 on leaving as-is.

[simonjayhawkins]

changing milestone to 1.3.5

[rhshadrach]

This no longer raises on master and produces the output

              FTE_OFF_DAYS ACTIVITY_CATEGORY
MONTH FTE                                   
April 0.75 2            10        Activity 1
      1.00 0             5        Activity 1

which looks correct to me. As mutation is not supported, I don't believe tests should be added and that this issue can be closed.

[simonjayhawkins]

This no longer raises on master and produces the output

fixed in commit: [d037ff6] REF: remove libreduction.apply_frame_axis0 (#42992)

[simonjayhawkins]

fixed in commit: [d037ff6] REF: remove libreduction.apply_frame_axis0 (#42992)

Because that PR made other behavioral changes #43206 (comment), we should probably rule out backporting in order to close this issue.

I don't believe tests should be added and that this issue can be closed.

will close but without a test this issue could potentially resurface, depending on fix to #43206