REGR: fix eval with inplace=True to correctly update column values inplace

[CloseChoice]

• closes REGR: DataFrame.eval not respecting inplace argument in pandas 1.4 #47449
• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added type annotations to new arguments/methods/functions.
• Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

The bad commit that causes this issue (see #43406) is the rootcause for multiple others, so am not sure if we want to tackle the overall problem bit by bit or if there is a generic solution to all of them.

[pep8speaks]

Hello @CloseChoice! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-08-19 08:09:31 UTC

[phofl]

We should not use this here. Please use loc for this.

[CloseChoice]

Thanks, that helps a lot.

[phofl]

Could you add a whatsnew? Is there a suitable class for this test?

The root cause was intended, so we have to fix them where they occur.

[CloseChoice]

Could you add a whatsnew? Is there a suitable class for this test?

The root cause was intended, so we have to fix them where they occur.

Thanks for the answer. I added a whatsnew entry.

[phofl]

This is not ideal. What could target be here? Dict, series, DataFrame, something else?

[CloseChoice]

Nope, basically these 3. Used ´isinstance(target, (DataFrame, Series))` instead

[jorisvandenbossche]

@CloseChoice I started converting the suggestion at #47449 (comment) into a PR, missing that you already opened one with a very similar fix! I hope you don't mind, but so I merged some of the changes I started doing in your PR:

• Also checking for inplace in eval.py (when not doing inplace, I think the target[assigner] = ret without loc is faster), as suggested in REGR: DataFrame.eval not respecting inplace argument in pandas 1.4 #47449 (comment)). In addition I also replaced (Series, DataFrame) with NDFrame
• I slightly updated the test (used some simpler (less) values), and also added a check for a viewing Series to be updated (in addition to viewing DataFrame)

[phofl]

Is it possible to get there with a MultiIndex? Similar to #47774 where the dimension reduction bothers us

[jorisvandenbossche]

I am not sure if eval supports referring (or assigning) to MultiIndex columns (I also don't directly see a test in test_eval.py that involved MultiIndex). I tried a few things that all fail:

# those two give syntax error when parsing the ast
df.eval("('A', 'a') = ('A', 'b') + ('B', 'a')")
df.eval("\"('A', 'a')\" = \"('A', 'b')\" + \"('B', 'a')\"")
# those two each give another error
df.eval("D = ('A', 'b') + ('B', 'a')") 
df.eval("D = \"('A', 'b')\" + \"('B', 'a')\"")

If not using existing column names, but just assigning a new column, that still seems to work with this PR:

In [1]: df = DataFrame({("A", "a"): [1, 2, 3], ("A", "b"): [4, 5, 6], ("B", "a"): [7, 8, 9]})

In [2]: df
Out[2]: 
   A     B
   a  b  a
0  1  4  7
1  2  5  8
2  3  6  9

In [3]: df.eval("D = 1")
Out[3]: 
   A     B  D
   a  b  a   
0  1  4  7  1
1  2  5  8  1
2  3  6  9  1

In [4]: df.eval("E = 1", inplace=True)

In [5]: df
Out[5]: 
   A     B  E
   a  b  a   
0  1  4  7  1
1  2  5  8  1
2  3  6  9  1

[phofl]

Thanks for checking

[CloseChoice]

@jorisvandenbossche I don't mind at all. Thanks for picking this up and improving upon it.

[phofl]

Greenish, merging

[phofl]

thx @CloseChoice