From dc210b60df8e3fbed542377d382fc71cfbf2b40f Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Mon, 30 Oct 2023 13:23:17 +0000 Subject: [PATCH 01/10] Revert _constructor check --- pandas/core/frame.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index f37be37f37693..f69dbab7efcdb 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -646,7 +646,7 @@ def _constructor(self) -> Callable[..., DataFrame]: return DataFrame def _constructor_from_mgr(self, mgr, axes): - if self._constructor is DataFrame: + if type(self) is DataFrame: # we are pandas.DataFrame (or a subclass that doesn't override _constructor) return self._from_mgr(mgr, axes=axes) else: From 47bb5de2532118670b44ffa59ea1659f87b714f1 Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Mon, 30 Oct 2023 13:47:34 +0000 Subject: [PATCH 02/10] Revert other constructor checks, allow warning in test_reindex_like_subclass --- pandas/core/frame.py | 2 +- pandas/core/series.py | 6 ++---- pandas/tests/frame/methods/test_reindex_like.py | 3 +++ 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index f69dbab7efcdb..ea4544b9d8440 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -659,7 +659,7 @@ def _sliced_from_mgr(self, mgr, axes) -> Series: return Series._from_mgr(mgr, axes) def _constructor_sliced_from_mgr(self, mgr, axes): - if self._constructor_sliced is Series: + if type(self) is DataFrame: ser = self._sliced_from_mgr(mgr, axes) ser._name = None # caller is responsible for setting real name return ser diff --git a/pandas/core/series.py b/pandas/core/series.py index c5f622a113258..2d6759f42c313 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -632,7 +632,7 @@ def _constructor(self) -> Callable[..., Series]: return Series def _constructor_from_mgr(self, mgr, axes): - if self._constructor is Series: + if type(self) is Series: # we are pandas.Series (or a subclass that doesn't override _constructor) ser = self._from_mgr(mgr, axes=axes) ser._name = None # caller is responsible for setting real name @@ -657,9 +657,7 @@ def _expanddim_from_mgr(self, mgr, axes) -> DataFrame: return DataFrame._from_mgr(mgr, axes=mgr.axes) def _constructor_expanddim_from_mgr(self, mgr, axes): - from pandas.core.frame import DataFrame - - if self._constructor_expanddim is DataFrame: + if type(self) is Series: return self._expanddim_from_mgr(mgr, axes) assert axes is mgr.axes return self._constructor_expanddim(mgr) diff --git a/pandas/tests/frame/methods/test_reindex_like.py b/pandas/tests/frame/methods/test_reindex_like.py index ce68ec28eec3d..85d833de93922 100644 --- a/pandas/tests/frame/methods/test_reindex_like.py +++ b/pandas/tests/frame/methods/test_reindex_like.py @@ -27,6 +27,9 @@ def test_reindex_like_methods(self, method, expected_values): result = df.reindex_like(df, method=method, tolerance=[0, 0, 0, 0]) tm.assert_frame_equal(df, result) + @pytest.mark.filterwarnings( + "ignore:Passing a BlockManager|Passing a SingleBlockManager:DeprecationWarning" + ) def test_reindex_like_subclass(self): # https://github.com/pandas-dev/pandas/issues/31925 class MyDataFrame(DataFrame): From 6c5086d1be8baa586b1ad13fb5583a42fd1acbba Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Tue, 31 Oct 2023 15:15:11 +0000 Subject: [PATCH 03/10] Add tests --- pandas/tests/frame/test_subclass.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pandas/tests/frame/test_subclass.py b/pandas/tests/frame/test_subclass.py index 55bfefaeb53a9..7fa1ab56ae225 100644 --- a/pandas/tests/frame/test_subclass.py +++ b/pandas/tests/frame/test_subclass.py @@ -773,3 +773,21 @@ def test_constructor_with_metadata(): ) subset = df[["A", "B"]] assert isinstance(subset, MySubclassWithMetadata) + + +class SimpleSubClass(DataFrame): + """A subclass of DataFrame that does not define a constructor.""" + + +class TestSubclassWithoutConstructor: + def test_copy(self): + expected = DataFrame({"a": [1, 2, 3]}) + result = SimpleSubClass(expected).copy() + + tm.assert_frame_equal(result, expected) + + def test_groupby(self): + df = SimpleSubClass(DataFrame({"a": [1, 2, 3]})) + + for _, v in df.groupby("a"): + assert isinstance(v, DataFrame) From 3bf8c8af272379de3821d89a5ca94ffc86c8fc96 Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Wed, 8 Nov 2023 15:02:45 +0000 Subject: [PATCH 04/10] Release note --- doc/source/whatsnew/v2.1.3.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v2.1.3.rst b/doc/source/whatsnew/v2.1.3.rst index 31ab01f171b4a..6413e16afd800 100644 --- a/doc/source/whatsnew/v2.1.3.rst +++ b/doc/source/whatsnew/v2.1.3.rst @@ -13,7 +13,7 @@ including other versions of pandas. Fixed regressions ~~~~~~~~~~~~~~~~~ -- +- Fixed infinite recursion from operations that return a new object on some DataFrame subclasses (:issue:`55763`) - .. --------------------------------------------------------------------------- From 123934dbf710331bba1be3b425375f68abdbe9c4 Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Wed, 8 Nov 2023 15:20:28 +0000 Subject: [PATCH 05/10] Add Series test, accept suggestion --- pandas/core/frame.py | 2 +- pandas/core/series.py | 2 +- pandas/tests/frame/test_subclass.py | 18 ++++++++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index ea4544b9d8440..61f188dad8c25 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -648,7 +648,7 @@ def _constructor(self) -> Callable[..., DataFrame]: def _constructor_from_mgr(self, mgr, axes): if type(self) is DataFrame: # we are pandas.DataFrame (or a subclass that doesn't override _constructor) - return self._from_mgr(mgr, axes=axes) + return DataFrame._from_mgr(mgr, axes=axes) else: assert axes is mgr.axes return self._constructor(mgr) diff --git a/pandas/core/series.py b/pandas/core/series.py index ba52d81285860..47cacc8162a39 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -637,7 +637,7 @@ def _constructor(self) -> Callable[..., Series]: def _constructor_from_mgr(self, mgr, axes): if type(self) is Series: # we are pandas.Series (or a subclass that doesn't override _constructor) - ser = self._from_mgr(mgr, axes=axes) + ser = Series._from_mgr(mgr, axes=axes) ser._name = None # caller is responsible for setting real name return ser else: diff --git a/pandas/tests/frame/test_subclass.py b/pandas/tests/frame/test_subclass.py index 7fa1ab56ae225..0bdb989a5f413 100644 --- a/pandas/tests/frame/test_subclass.py +++ b/pandas/tests/frame/test_subclass.py @@ -775,19 +775,29 @@ def test_constructor_with_metadata(): assert isinstance(subset, MySubclassWithMetadata) -class SimpleSubClass(DataFrame): +class SimpleDataFrameSubClass(DataFrame): """A subclass of DataFrame that does not define a constructor.""" +class SimpleSeriesSubClass(Series): + """A subclass of Series that does not define a constructor.""" + + class TestSubclassWithoutConstructor: - def test_copy(self): + def test_copy_df(self): expected = DataFrame({"a": [1, 2, 3]}) - result = SimpleSubClass(expected).copy() + result = SimpleDataFrameSubClass(expected).copy() tm.assert_frame_equal(result, expected) + def test_copy_series(self): + expected = Series([1, 2, 3]) + result = SimpleSeriesSubClass(expected).copy() + + tm.assert_series_equal(result, expected) + def test_groupby(self): - df = SimpleSubClass(DataFrame({"a": [1, 2, 3]})) + df = SimpleDataFrameSubClass(DataFrame({"a": [1, 2, 3]})) for _, v in df.groupby("a"): assert isinstance(v, DataFrame) From c61c6907eef95c3bf9b9184f584f595f44b7684a Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Wed, 8 Nov 2023 16:25:06 +0000 Subject: [PATCH 06/10] Revert specific instance check, use of class method now catches this --- pandas/core/frame.py | 2 +- pandas/core/series.py | 2 +- pandas/tests/frame/methods/test_reindex_like.py | 3 --- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 61f188dad8c25..5f523f9639f44 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -646,7 +646,7 @@ def _constructor(self) -> Callable[..., DataFrame]: return DataFrame def _constructor_from_mgr(self, mgr, axes): - if type(self) is DataFrame: + if self._constructor is DataFrame: # we are pandas.DataFrame (or a subclass that doesn't override _constructor) return DataFrame._from_mgr(mgr, axes=axes) else: diff --git a/pandas/core/series.py b/pandas/core/series.py index 47cacc8162a39..a02e3bb4f6e35 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -635,7 +635,7 @@ def _constructor(self) -> Callable[..., Series]: return Series def _constructor_from_mgr(self, mgr, axes): - if type(self) is Series: + if self._constructor is Series: # we are pandas.Series (or a subclass that doesn't override _constructor) ser = Series._from_mgr(mgr, axes=axes) ser._name = None # caller is responsible for setting real name diff --git a/pandas/tests/frame/methods/test_reindex_like.py b/pandas/tests/frame/methods/test_reindex_like.py index 85d833de93922..ce68ec28eec3d 100644 --- a/pandas/tests/frame/methods/test_reindex_like.py +++ b/pandas/tests/frame/methods/test_reindex_like.py @@ -27,9 +27,6 @@ def test_reindex_like_methods(self, method, expected_values): result = df.reindex_like(df, method=method, tolerance=[0, 0, 0, 0]) tm.assert_frame_equal(df, result) - @pytest.mark.filterwarnings( - "ignore:Passing a BlockManager|Passing a SingleBlockManager:DeprecationWarning" - ) def test_reindex_like_subclass(self): # https://github.com/pandas-dev/pandas/issues/31925 class MyDataFrame(DataFrame): From bac1e951e5c85031f753f29a4d5bf4f09cfe68de Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Wed, 8 Nov 2023 16:47:47 +0000 Subject: [PATCH 07/10] Fix type checks in tests --- pandas/tests/frame/test_subclass.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pandas/tests/frame/test_subclass.py b/pandas/tests/frame/test_subclass.py index 0bdb989a5f413..31613264156f3 100644 --- a/pandas/tests/frame/test_subclass.py +++ b/pandas/tests/frame/test_subclass.py @@ -788,6 +788,9 @@ def test_copy_df(self): expected = DataFrame({"a": [1, 2, 3]}) result = SimpleDataFrameSubClass(expected).copy() + assert ( + type(result) is DataFrame + ) # assert_frame_equal only checks isinstance(lhs, type(rhs)) tm.assert_frame_equal(result, expected) def test_copy_series(self): @@ -800,4 +803,4 @@ def test_groupby(self): df = SimpleDataFrameSubClass(DataFrame({"a": [1, 2, 3]})) for _, v in df.groupby("a"): - assert isinstance(v, DataFrame) + assert type(v) is DataFrame From 885aadb9e413158c77ac891da89e01ed77c2fc5c Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Wed, 8 Nov 2023 17:58:19 +0100 Subject: [PATCH 08/10] Update pandas/core/frame.py Co-authored-by: Isaac Virshup --- pandas/core/frame.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 5f523f9639f44..613b6a39841a2 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -659,7 +659,7 @@ def _sliced_from_mgr(self, mgr, axes) -> Series: return Series._from_mgr(mgr, axes) def _constructor_sliced_from_mgr(self, mgr, axes): - if type(self) is DataFrame: + if self._constructor_sliced is Series: ser = self._sliced_from_mgr(mgr, axes) ser._name = None # caller is responsible for setting real name return ser From 3cb17f158b6d29e9e47c4ca1e2153dbd3fc29bb8 Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Wed, 8 Nov 2023 17:58:28 +0100 Subject: [PATCH 09/10] Update pandas/core/series.py Co-authored-by: Isaac Virshup --- pandas/core/series.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pandas/core/series.py b/pandas/core/series.py index a02e3bb4f6e35..368090a37d977 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -660,7 +660,9 @@ def _expanddim_from_mgr(self, mgr, axes) -> DataFrame: return DataFrame._from_mgr(mgr, axes=mgr.axes) def _constructor_expanddim_from_mgr(self, mgr, axes): - if type(self) is Series: + from pandas.core.frame import DataFrame + + if self._constructor_expanddim is DataFrame: return self._expanddim_from_mgr(mgr, axes) assert axes is mgr.axes return self._constructor_expanddim(mgr) From 22c33c3370d601c2aa5b87da9f5cb7bfef131169 Mon Sep 17 00:00:00 2001 From: Isaac Virshup Date: Wed, 8 Nov 2023 17:13:28 +0000 Subject: [PATCH 10/10] Add test for Series.to_frame --- pandas/tests/frame/test_subclass.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pandas/tests/frame/test_subclass.py b/pandas/tests/frame/test_subclass.py index 31613264156f3..f19e31002c877 100644 --- a/pandas/tests/frame/test_subclass.py +++ b/pandas/tests/frame/test_subclass.py @@ -799,6 +799,16 @@ def test_copy_series(self): tm.assert_series_equal(result, expected) + def test_series_to_frame(self): + orig = Series([1, 2, 3]) + expected = orig.to_frame() + result = SimpleSeriesSubClass(orig).to_frame() + + assert ( + type(result) is DataFrame + ) # assert_frame_equal only checks isinstance(lhs, type(rhs)) + tm.assert_frame_equal(result, expected) + def test_groupby(self): df = SimpleDataFrameSubClass(DataFrame({"a": [1, 2, 3]}))