Repository: pandas-dev/pandas
Pull request #56761 (2024-01-07 09:57:38)

BUG: fix subclass metadata preservation in groupby column selection
This is a small regression that was introduced with https://github.com/pandas-dev/pandas/pull/51090 (in pandas 2.0): before that PR, the column subselection in the groupby code was done with a standard `__getitem__` call, while the PR introduced an optimized helper `_getitem_nocopy` to avoid the copy that a standard `__getitem__` does. However, that new helper forgot to add a `__finalize__` call after `_constructor` from a manager.

The reason we noticed this in geopandas (https://github.com/geopandas/geopandas/pull/3130) was to update our tests for the `apply` deprecation to include group columns (for which one alternative is to specifically select the non-group columns, but so that didn't work with recent pandas versions)


Patch:
diff --git a/doc/source/whatsnew/v2.2.0.rst b/doc/source/whatsnew/v2.2.0.rst
index 0b04a1d313..2b436bc5d1 100644
--- a/doc/source/whatsnew/v2.2.0.rst
+++ b/doc/source/whatsnew/v2.2.0.rst
@@ -873,6 +873,7 @@ Groupby/resample/rolling
 - Bug in :meth:`DataFrame.asfreq` and :meth:`Series.asfreq` with a :class:`DatetimeIndex` with non-nanosecond resolution incorrectly converting to nanosecond resolution (:issue:`55958`)
 - Bug in :meth:`DataFrame.ewm` when passed ``times`` with non-nanosecond ``datetime64`` or :class:`DatetimeTZDtype` dtype (:issue:`56262`)
 - Bug in :meth:`DataFrame.groupby` and :meth:`Series.groupby` where grouping by a combination of ``Decimal`` and NA values would fail when ``sort=True`` (:issue:`54847`)
+- Bug in :meth:`DataFrame.groupby` for DataFrame subclasses when selecting a subset of columns to apply the function to (:issue:`56761`)
 - Bug in :meth:`DataFrame.resample` not respecting ``closed`` and ``label`` arguments for :class:`~pandas.tseries.offsets.BusinessDay` (:issue:`55282`)
 - Bug in :meth:`DataFrame.resample` when resampling on a :class:`ArrowDtype` of ``pyarrow.timestamp`` or ``pyarrow.duration`` type (:issue:`55989`)
 - Bug in :meth:`DataFrame.resample` where bin edges were not correct for :class:`~pandas.tseries.offsets.BusinessDay` (:issue:`55281`)
diff --git a/doc/source/whatsnew/v2.3.0.rst b/doc/source/whatsnew/v2.3.0.rst
index 1f1b0c7d71..c0692dba32 100644
--- a/doc/source/whatsnew/v2.3.0.rst
+++ b/doc/source/whatsnew/v2.3.0.rst
@@ -108,6 +108,8 @@ Performance improvements
 
 Bug fixes
 ~~~~~~~~~
+- Fixed bug in :meth:`Series.diff` allowing non-integer values for the ``periods`` argument. (:issue:`56607`)
+
 
 Categorical
 ^^^^^^^^^^^
diff --git a/pandas/core/algorithms.py b/pandas/core/algorithms.py
index 76fdcefd03..128477dac5 100644
--- a/pandas/core/algorithms.py
+++ b/pandas/core/algorithms.py
@@ -47,6 +47,7 @@ from pandas.core.dtypes.common import (
     is_complex_dtype,
     is_dict_like,
     is_extension_array_dtype,
+    is_float,
     is_float_dtype,
     is_integer,
     is_integer_dtype,
@@ -1361,7 +1362,12 @@ def diff(arr, n: int, axis: AxisInt = 0):
     shifted
     """
 
-    n = int(n)
+    # added a check on the integer value of period
+    # see https://github.com/pandas-dev/pandas/issues/56607
+    if not lib.is_integer(n):
+        if not (is_float(n) and n.is_integer()):
+            raise ValueError("periods must be an integer")
+        n = int(n)
     na = np.nan
     dtype = arr.dtype
 
diff --git a/pandas/core/frame.py b/pandas/core/frame.py
index 8db437ccec..021c7b74ad 100644
--- a/pandas/core/frame.py
+++ b/pandas/core/frame.py
@@ -4016,7 +4016,9 @@ class DataFrame(NDFrame, OpsMixin):
             copy=False,
             only_slice=True,
         )
-        return self._constructor_from_mgr(new_mgr, axes=new_mgr.axes)
+        result = self._constructor_from_mgr(new_mgr, axes=new_mgr.axes)
+        result = result.__finalize__(self)
+        return result
 
     def __getitem__(self, key):
         check_dict_or_set_indexers(key)
diff --git a/pandas/core/series.py b/pandas/core/series.py
index 1f9ac85114..90073e21cf 100644
--- a/pandas/core/series.py
+++ b/pandas/core/series.py
@@ -72,6 +72,7 @@ from pandas.core.dtypes.cast import (
 )
 from pandas.core.dtypes.common import (
     is_dict_like,
+    is_float,
     is_integer,
     is_iterator,
     is_list_like,
@@ -3102,6 +3103,9 @@ class Series(base.IndexOpsMixin, NDFrame):  # type: ignore[misc]
         --------
         {examples}
         """
+        if not lib.is_integer(periods):
+            if not (is_float(periods) and periods.is_integer()):
+                raise ValueError("periods must be an integer")
         result = algorithms.diff(self._values, periods)
         return self._constructor(result, index=self.index, copy=False).__finalize__(
             self, method="diff"


Test patch:
diff --git a/pandas/tests/groupby/test_groupby_subclass.py b/pandas/tests/groupby/test_groupby_subclass.py
index bf809bd5db..17ef6ee913 100644
--- a/pandas/tests/groupby/test_groupby_subclass.py
+++ b/pandas/tests/groupby/test_groupby_subclass.py
@@ -69,6 +69,7 @@ def test_groupby_preserves_metadata():
     def func(group):
         assert isinstance(group, tm.SubclassedDataFrame)
         assert hasattr(group, "testattr")
+        assert group.testattr == "hello"
         return group.testattr
 
     msg = "DataFrameGroupBy.apply operated on the grouping columns"
@@ -79,6 +80,13 @@ def test_groupby_preserves_metadata():
     expected = tm.SubclassedSeries(["hello"] * 3, index=Index([7, 8, 9], name="c"))
     tm.assert_series_equal(result, expected)
 
+    result = custom_df.groupby("c").apply(func, include_groups=False)
+    tm.assert_series_equal(result, expected)
+
+    # https://github.com/pandas-dev/pandas/pull/56761
+    result = custom_df.groupby("c")[["a", "b"]].apply(func)
+    tm.assert_series_equal(result, expected)
+
     def func2(group):
         assert isinstance(group, tm.SubclassedSeries)
         assert hasattr(group, "testattr")
diff --git a/pandas/tests/series/methods/test_diff.py b/pandas/tests/series/methods/test_diff.py
index 18de81a927..a46389087f 100644
--- a/pandas/tests/series/methods/test_diff.py
+++ b/pandas/tests/series/methods/test_diff.py
@@ -10,6 +10,11 @@ import pandas._testing as tm
 
 
 class TestSeriesDiff:
+    def test_diff_series_requires_integer(self):
+        series = Series(np.random.default_rng(2).standard_normal(2))
+        with pytest.raises(ValueError, match="periods must be an integer"):
+            series.diff(1.5)
+
     def test_diff_np(self):
         # TODO(__array_function__): could make np.diff return a Series
         #  matching ser.diff()