-
Notifications
You must be signed in to change notification settings - Fork 28.1k
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
[SPARK-48295][PS] Turn on compute.ops_on_diff_frames
by default
#46602
[SPARK-48295][PS] Turn on compute.ops_on_diff_frames
by default
#46602
Conversation
will add it to release note soon |
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.
Otherwise LGTM when test passing
>>> with ps.option_context("compute.ops_on_diff_frames", False): | ||
... ps.DataFrame(data=sdf, index=pd.Index([0, 1, 2])) |
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 maybe now we can just remove the negative cases from the doctests and keep only UTs??
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 am not sure, @HyukjinKwon wdyt?
>>> with ps.option_context("compute.ops_on_diff_frames", False): | ||
... ps.DataFrame(data=sdf, index=ps.Index([0, 1, 2])) |
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.
ditto and all belows?
2458d35
to
1367024
Compare
Merged to master. |
What changes were proposed in this pull request?
Turn on
compute.ops_on_diff_frames
by defaultWhy are the changes needed?
1, in most cases, this config need to be turned on to enable computation with different dataframes;
2, enable
compute.ops_on_diff_frames
should not break any workloads, it should only enable more;Does this PR introduce any user-facing change?
yes, this config is turned on by default
How was this patch tested?
updated tests
Was this patch authored or co-authored using generative AI tooling?
no