-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
DataFrame.eval error fixed #59145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DataFrame.eval error fixed #59145
Conversation
@@ -218,3 +217,12 @@ def reconstruct_object(typ, obj, axes, dtype): | |||
ret_value = np.array([ret_value]).astype(res_t) | |||
|
|||
return ret_value | |||
|
|||
# Test with the DataFrame and multiline expression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move the test to a proper test file? I think pandas/tests/test_expressions.py
is probably the best place for it.
@@ -144,8 +145,6 @@ def _align_core(terms): | |||
obj = ti.reindex(reindexer, axis=axis) | |||
terms[i].update(obj) | |||
|
|||
terms[i].update(terms[i].value.values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this line does fix the bug, nice job! But I'm curious about potential side effects: this line of code converts terms[i].value
from pd.Series
to np.ndarray
, and removing it might affect the behavior of terms
in whatever context it is used. Are the test failures related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure @chaoyihu I have added the test in the respective file
But here what you want me to do further in this terms[i].value ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should wait and see if it passes the checks after this revision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick response! I viewed your changes and left new comments.
The check failures seem to have been caused by the import line you added to the beginning of align.py
- from __future__
imports must occur at the beginning of the file.
You probably should also rebase your branch on main
, since your branch is significantly behind.
For now, I do not think you need to do anything further with terms[i]
. Can you revise the code, do a rebase, then push again and wait to see if it passes the checks?
If it does, someone with write access will likely come to inspect and merge your code.
I have moved the test in the respective file |
@@ -2,6 +2,7 @@ | |||
Core eval alignment algorithms. | |||
""" | |||
|
|||
import pandas as pd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this import line.
@@ -218,3 +218,12 @@ def reconstruct_object(typ, obj, axes, dtype): | |||
ret_value = np.array([ret_value]).astype(res_t) | |||
|
|||
return ret_value | |||
|
|||
# Test with the DataFrame and multiline expression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't forget to remove this block of code from align.py
, since you've added it to the test file.
@@ -2,6 +2,7 @@ | |||
import re | |||
|
|||
import numpy as np | |||
import pandas as pd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this line.
# Define expected values manually or calculate based on expectation | ||
expected_first_ret = df['first'] / df['pre'].fillna(df['first']) - 1.0 | ||
expected_last_ret = df['last'] / df['pre'].fillna(df['first']) - 1.0 | ||
expected = pd.DataFrame({"first_ret": expected_first_ret, "last_ret": expected_last_ret}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use DataFrame
instead of pd.DataFrame
.
|
||
# New test added here | ||
def test_multiline_expression(self): | ||
df = pd.DataFrame({"first": [9.76, 9.76, 9.76], "last": [9.76, 9.76, 9.76], "pre": [9.75, 9.76, 9.76]}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use DataFrame
instead of pd.DataFrame
.
I am closing this PR |
DataFrame.eval
fails with TypeError with multiline expr but works wheneval
line by line #59062 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.