From 81f64afde163fdbbbc548de06d8c5adc6a5bb895 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Sat, 3 Aug 2024 23:35:19 -0400 Subject: [PATCH 1/8] Add enhancement to v3.0.0 --- doc/source/whatsnew/v3.0.0.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v3.0.0.rst b/doc/source/whatsnew/v3.0.0.rst index e3c4e69db7cbd..5bd00e608077f 100644 --- a/doc/source/whatsnew/v3.0.0.rst +++ b/doc/source/whatsnew/v3.0.0.rst @@ -50,6 +50,7 @@ Other enhancements - :meth:`DataFrame.pivot_table` and :func:`pivot_table` now allow the passing of keyword arguments to ``aggfunc`` through ``**kwargs`` (:issue:`57884`) - :meth:`Series.cummin` and :meth:`Series.cummax` now supports :class:`CategoricalDtype` (:issue:`52335`) - :meth:`Series.plot` now correctly handle the ``ylabel`` parameter for pie charts, allowing for explicit control over the y-axis label (:issue:`58239`) +- :meth:`pandas.concat` will raise a ``ValueError`` when the combination of given arguments don't make sense (:issue:`59274`) - Restore support for reading Stata 104-format and enable reading 103-format dta files (:issue:`58554`) - Support reading Stata 102-format (Stata 1) dta files (:issue:`58978`) - Support reading Stata 110-format (Stata 7) dta files (:issue:`47176`) From 7c67b231ce8179c056a5163ed7a8bd8775314011 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Sat, 3 Aug 2024 23:37:29 -0400 Subject: [PATCH 2/8] Raise error when combination of given values don't make sense --- pandas/core/reshape/concat.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pandas/core/reshape/concat.py b/pandas/core/reshape/concat.py index 6836ba3f65691..33fd9a6c8bc40 100644 --- a/pandas/core/reshape/concat.py +++ b/pandas/core/reshape/concat.py @@ -379,6 +379,12 @@ def concat( 0 1 2 1 3 4 """ + if ignore_index and keys is not None: + raise ValueError( + "Setting ignore_index to true and providing key values are " + "counterproductive. Either should be used." + ) + if copy is not lib.no_default: warnings.warn( "The copy keyword is deprecated and will be removed in a future " From 6a5c71fd8db07e0fed877378198893b1615e4837 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Sat, 3 Aug 2024 23:40:57 -0400 Subject: [PATCH 3/8] Add test --- pandas/tests/reshape/concat/test_concat.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pandas/tests/reshape/concat/test_concat.py b/pandas/tests/reshape/concat/test_concat.py index b2caa1fadd1a5..c3d28bab60dcc 100644 --- a/pandas/tests/reshape/concat/test_concat.py +++ b/pandas/tests/reshape/concat/test_concat.py @@ -939,3 +939,15 @@ def test_concat_with_series_and_frame_returns_rangeindex_columns(): result = concat([ser, df]) expected = DataFrame([0, 1, 2], index=[0, 0, 1]) tm.assert_frame_equal(result, expected, check_column_type=True) + + +def test_concat_with_moot_combination_of_arguments(): + df1 = DataFrame([[0]]) + df2 = DataFrame([[42]]) + + msg = ( + "Setting ignore_index to true and providing key values are " + "counterproductive. Either should be used." + ) + with pytest.raises(ValueError, match=msg): + concat([df1, df2], keys=["df1", "df2"], ignore_index=True) From c230b34b28d889100ff9879af2ba322bd4bf0820 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Mon, 5 Aug 2024 22:35:59 -0400 Subject: [PATCH 4/8] Update doc/source/whatsnew/v3.0.0.rst Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> --- doc/source/whatsnew/v3.0.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v3.0.0.rst b/doc/source/whatsnew/v3.0.0.rst index 5bd00e608077f..209a841ac9ca6 100644 --- a/doc/source/whatsnew/v3.0.0.rst +++ b/doc/source/whatsnew/v3.0.0.rst @@ -50,7 +50,7 @@ Other enhancements - :meth:`DataFrame.pivot_table` and :func:`pivot_table` now allow the passing of keyword arguments to ``aggfunc`` through ``**kwargs`` (:issue:`57884`) - :meth:`Series.cummin` and :meth:`Series.cummax` now supports :class:`CategoricalDtype` (:issue:`52335`) - :meth:`Series.plot` now correctly handle the ``ylabel`` parameter for pie charts, allowing for explicit control over the y-axis label (:issue:`58239`) -- :meth:`pandas.concat` will raise a ``ValueError`` when the combination of given arguments don't make sense (:issue:`59274`) +- :meth:`pandas.concat` will raise a ``ValueError`` when ``ignore_index=True`` and ``keys`` is not ``None`` (:issue:`59274`) - Restore support for reading Stata 104-format and enable reading 103-format dta files (:issue:`58554`) - Support reading Stata 102-format (Stata 1) dta files (:issue:`58978`) - Support reading Stata 110-format (Stata 7) dta files (:issue:`47176`) From 36e66191ad8438478c6fabf3d27d2b1b8ce865ab Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Mon, 5 Aug 2024 22:36:26 -0400 Subject: [PATCH 5/8] Update pandas/core/reshape/concat.py Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> --- pandas/core/reshape/concat.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pandas/core/reshape/concat.py b/pandas/core/reshape/concat.py index 33fd9a6c8bc40..c005a1ce26e4b 100644 --- a/pandas/core/reshape/concat.py +++ b/pandas/core/reshape/concat.py @@ -381,8 +381,7 @@ def concat( """ if ignore_index and keys is not None: raise ValueError( - "Setting ignore_index to true and providing key values are " - "counterproductive. Either should be used." + f"Cannot set {ignore_index=} and specify keys. Either should be used." ) if copy is not lib.no_default: From 346b30b747876557e9c72b2abd4eeeac344e6091 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Mon, 5 Aug 2024 22:37:07 -0400 Subject: [PATCH 6/8] Update pandas/tests/reshape/concat/test_concat.py Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> --- pandas/tests/reshape/concat/test_concat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/reshape/concat/test_concat.py b/pandas/tests/reshape/concat/test_concat.py index c3d28bab60dcc..bbbcb58dda8d7 100644 --- a/pandas/tests/reshape/concat/test_concat.py +++ b/pandas/tests/reshape/concat/test_concat.py @@ -941,7 +941,7 @@ def test_concat_with_series_and_frame_returns_rangeindex_columns(): tm.assert_frame_equal(result, expected, check_column_type=True) -def test_concat_with_moot_combination_of_arguments(): +def test_concat_with_moot_ignore_index_and_keys): df1 = DataFrame([[0]]) df2 = DataFrame([[42]]) From cd320be335cab19de2b0bcfcb7c8bffab1a77e09 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Sat, 10 Aug 2024 08:47:22 -0400 Subject: [PATCH 7/8] Update pandas/tests/reshape/concat/test_concat.py Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> --- pandas/tests/reshape/concat/test_concat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/reshape/concat/test_concat.py b/pandas/tests/reshape/concat/test_concat.py index bbbcb58dda8d7..37b97580fcad7 100644 --- a/pandas/tests/reshape/concat/test_concat.py +++ b/pandas/tests/reshape/concat/test_concat.py @@ -941,7 +941,7 @@ def test_concat_with_series_and_frame_returns_rangeindex_columns(): tm.assert_frame_equal(result, expected, check_column_type=True) -def test_concat_with_moot_ignore_index_and_keys): +def test_concat_with_moot_ignore_index_and_keys(): df1 = DataFrame([[0]]) df2 = DataFrame([[42]]) From cb82613ccde99fafc585354b3730cc2fbc03b361 Mon Sep 17 00:00:00 2001 From: Jay Ahn Date: Sat, 10 Aug 2024 09:21:22 -0400 Subject: [PATCH 8/8] Fix test --- pandas/tests/reshape/concat/test_concat.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pandas/tests/reshape/concat/test_concat.py b/pandas/tests/reshape/concat/test_concat.py index 37b97580fcad7..8af224f1ad64f 100644 --- a/pandas/tests/reshape/concat/test_concat.py +++ b/pandas/tests/reshape/concat/test_concat.py @@ -945,9 +945,8 @@ def test_concat_with_moot_ignore_index_and_keys(): df1 = DataFrame([[0]]) df2 = DataFrame([[42]]) - msg = ( - "Setting ignore_index to true and providing key values are " - "counterproductive. Either should be used." - ) + ignore_index = True + keys = ["df1", "df2"] + msg = f"Cannot set {ignore_index=} and specify keys. Either should be used." with pytest.raises(ValueError, match=msg): - concat([df1, df2], keys=["df1", "df2"], ignore_index=True) + concat([df1, df2], keys=keys, ignore_index=ignore_index)