-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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-32686][PYTHON] Un-deprecate inferring DataFrame schema from list of dict #29510
Conversation
cc @HyukjinKwon |
Test build #127755 has finished for PR 29510 at commit
|
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.
Looks good assuming from the history:
- Python 2.7's dict does not keep the order, which is fixed in Python 3 and PySpark at SPARK-29748
- It was deprecated when
Row
API was introduced at 51aa135 in SPARK-2010. I think we now target Python usability and only allowingRow
doesn't look Python friendly.
cc @BryanCutler fyi |
Yeah, this deprecation has been around for ~6 years. I wonder if we should check with dev@ to make sure there are no surprising consequences of un-deprecating it? |
Test build #127845 has finished for PR 29510 at commit
|
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.
LGTM
Test build #127851 has finished for PR 29510 at commit
|
merged to master, thanks @nchammas ! |
What changes were proposed in this pull request?
As discussed in #29491 (comment) and in SPARK-32686, this PR un-deprecates Spark's ability to infer a DataFrame schema from a list of dictionaries. The ability is Pythonic and matches functionality offered by Pandas.
Why are the changes needed?
This change clarifies to users that this behavior is supported and is not going away in the near future.
Does this PR introduce any user-facing change?
Yes. There used to be a
UserWarning
for this, but now there isn't.How was this patch tested?
I tested this manually.
Before:
After: