BUG: raise when doing arithmetic on DataFrame and list of iterables #37132

AlexKirko · 2020-10-15T08:15:43Z

closes Undocumented behavior change (1.0.5->1.1.0) when using arithmetic operations on dataframes #36702
tests 1 added / 1 passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Problem

We currently allow doing arithmetic on a DataFrame, and, for example, a list of Series. The latter gets broadcast with align_method_FRAME and this leads to weirdness.

Solution

We should raise. There is no definite answer to how such arithmetic should be handled, and the user should just make another DataFrame and add that to the frame instead of doing stuff like [Series, Series].

pandas/core/ops/__init__.py

jreback · 2020-10-15T12:28:55Z

doc/source/whatsnew/v1.2.0.rst

@@ -494,6 +494,7 @@ Other
 - Fixed metadata propagation in the :class:`Series.dt` and :class:`Series.str` accessors (:issue:`28283`)
 - Bug in :meth:`Index.union` behaving differently depending on whether operand is a :class:`Index` or other list-like (:issue:`36384`)
 - Passing an array with 2 or more dimensions to the :class:`Series` constructor now raises the more specific ``ValueError``, from a bare ``Exception`` previously (:issue:`35744`)
+- Bug in :class:`DataFrame` allowing arithmetic operations with list of array-likes with undefined results. Behavior changed to raising ``ValueError`` (:issue:`36702`)


put in Numeric section

pandas/tests/frame/test_arithmetic.py

pandas/core/ops/__init__.py

jreback · 2020-10-15T12:30:13Z

cc @jbrockmendel

jbrockmendel · 2020-10-15T18:11:33Z

pandas/tests/frame/test_arithmetic.py

@@ -1577,3 +1577,17 @@ def test_arith_reindex_with_duplicates():
    result = df1 + df2
    expected = pd.DataFrame([[np.nan, 0, 0]], columns=["first", "second", "second"])
    tm.assert_frame_equal(result, expected)
+
+
+@pytest.mark.parametrize("arg1", [pd.DataFrame({"x": [1, 2], "y": [1, 2]})])


if there's just one of these (and i think there only needs to be one), then just put it in the test function instead of parameter

Thanks for spotting this! Moved the DataFrame inside the test.

jreback

comment by @jbrockmendel ping on green

AlexKirko · 2020-10-16T09:04:36Z

Ran indexing benchmarks three times, no stable regressions on my machine.
I'm running the benchmark suite on Ubuntu inside Windows with plenty of free CPU and memory, but I still saw different 2-5 regressions / improvements during the three runs that I did. Sorry, don't have a dedicated machine for benchmarking, and my setup isn't always stable (probably because of all the junk in Windows grabbing and releasing resources).
We should be fine, since no regression or improvement held across runs.

AlexKirko · 2020-10-16T12:06:00Z

@jreback

Made the change suggested by @jbrockmendel , all green.

jreback · 2020-10-16T19:24:47Z

thanks @AlexKirko nice!

…andas-dev#37132)

vnmabus · 2021-07-10T21:06:45Z

Unfortunately this fix breaks test_arith_frame_with_scalar for my custom ExtensionArray. I have to say that my type is a bit "weird" in the sense that indexing it gives a object of the same type (like the string type in the standard library). This fix does not seem to like that very much.

jbrockmendel · 2021-07-10T21:38:15Z

@vnmabus can you give a link or example?

vnmabus · 2021-07-10T21:47:42Z

I have two extension arrays in https://github.com/GAA-UAM/scikit-fda, FDataBasis and FDataGrid. Both of them are used to represent a sample of functions (functional data). However, for simplicity, as I did not want to create a "just one function" object to represent a scalar value which won't be used much, I made __getitem__ with an integer key return an object of the same kind and length 1.

These extension arrays (mostly) work with Pandas, but this fix raises an exception, as they are "list like" and the elements (that is, the same class) are "array like". Yo can see the error here: https://github.com/GAA-UAM/scikit-fda/runs/3037343824.

jbrockmendel · 2021-07-11T17:22:31Z

Haven't looked at this too closely, but I think maybe if you make FDataAFoo.__iter__ yield the scalars (functions) that might avoid this particular problem. That would mean an inconsistency between list(fdata) vs [fdata[i] for i in range(len(fdata))] though.

vnmabus · 2021-07-11T17:25:23Z

The thing is that currently there isn't a separate class for the scalars. A scalar in my case is just an object of the same kind but with length one.

jbrockmendel · 2021-07-11T17:45:12Z

Yah I don't see any obvious way to avoid inconsistencies. If this is a serious problem, I'd suggest opening a new issue for dedicated discussion

AlexKirko added 6 commits October 14, 2020 18:26

TST: add test for raise

96e92b0

BUG: add initial bugfix, edit error message

79e98cd

DOC: add whatsnew

7a0e20c

CLN: run black pandas

0b461b1

CLN: fix flake8 errors

309b3db

Merge branch 'master' into add-list-to-df-raise

bb67158

AlexKirko commented Oct 15, 2020

View reviewed changes

pandas/core/ops/__init__.py Show resolved Hide resolved

CLN: remove unnecessary string split

e7561da

AlexKirko requested a review from jreback October 15, 2020 10:12

jreback added Numeric Operations Arithmetic, Comparison, and Logical operations Error Reporting Incorrect or improved errors from pandas labels Oct 15, 2020

jreback requested changes Oct 15, 2020

View reviewed changes

AlexKirko added 2 commits October 15, 2020 16:25

DOC: move whatsnew to Numeric section

a464f67

TST: add reverse op and one-element list to the test

0acac06

jbrockmendel reviewed Oct 15, 2020

View reviewed changes

jreback added this to the 1.2 milestone Oct 16, 2020

jreback requested changes Oct 16, 2020

View reviewed changes

AlexKirko added 2 commits October 16, 2020 12:07

TST: move DataFrame inside the test

26d6aba

restart checks

50fc0ee

AlexKirko requested a review from jreback October 16, 2020 12:06

jreback approved these changes Oct 16, 2020

View reviewed changes

jreback merged commit 9140647 into pandas-dev:master Oct 16, 2020

AlexKirko deleted the add-list-to-df-raise branch October 17, 2020 05:22

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Oct 26, 2020

BUG: raise when doing arithmetic on DataFrame and list of iterables (p…

28eb9b4

…andas-dev#37132)

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

BUG: raise when doing arithmetic on DataFrame and list of iterables (p…

326c192

…andas-dev#37132)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: raise when doing arithmetic on DataFrame and list of iterables #37132

BUG: raise when doing arithmetic on DataFrame and list of iterables #37132

AlexKirko commented Oct 15, 2020

jreback Oct 15, 2020

AlexKirko Oct 15, 2020

jreback commented Oct 15, 2020

jbrockmendel Oct 15, 2020

AlexKirko Oct 16, 2020

jreback left a comment

AlexKirko commented Oct 16, 2020 •

edited

Loading

AlexKirko commented Oct 16, 2020 •

edited

Loading

jreback commented Oct 16, 2020

vnmabus commented Jul 10, 2021 •

edited

Loading

jbrockmendel commented Jul 10, 2021

vnmabus commented Jul 10, 2021

jbrockmendel commented Jul 11, 2021

vnmabus commented Jul 11, 2021

jbrockmendel commented Jul 11, 2021

BUG: raise when doing arithmetic on DataFrame and list of iterables #37132

BUG: raise when doing arithmetic on DataFrame and list of iterables #37132

Conversation

AlexKirko commented Oct 15, 2020

Problem

Solution

jreback Oct 15, 2020

Choose a reason for hiding this comment

AlexKirko Oct 15, 2020

Choose a reason for hiding this comment

jreback commented Oct 15, 2020

jbrockmendel Oct 15, 2020

Choose a reason for hiding this comment

AlexKirko Oct 16, 2020

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

AlexKirko commented Oct 16, 2020 • edited Loading

AlexKirko commented Oct 16, 2020 • edited Loading

jreback commented Oct 16, 2020

vnmabus commented Jul 10, 2021 • edited Loading

jbrockmendel commented Jul 10, 2021

vnmabus commented Jul 10, 2021

jbrockmendel commented Jul 11, 2021

vnmabus commented Jul 11, 2021

jbrockmendel commented Jul 11, 2021

AlexKirko commented Oct 16, 2020 •

edited

Loading

AlexKirko commented Oct 16, 2020 •

edited

Loading

vnmabus commented Jul 10, 2021 •

edited

Loading