-
Notifications
You must be signed in to change notification settings - Fork 147
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
[FEAT] Add support for pyiceberg v0.7 #2594
Conversation
Gonna run these tests on pyiceberg==0.6.0 first to ensure no regressions, then update our requirements-dev to 0.7.0. Update: no regressions |
daft/dataframe/dataframe.py
Outdated
]: | ||
raise ValueError( | ||
f"Not all partition types are supported for writes. Following partitions cannot be written using pyarrow: {unsupported_partitions}." | ||
) |
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.
Wait, so partitioned write are supported? But certain transforms arent'?
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.
If that's the case, do we have tests yet for partitioned writes on tables with "simple" partition transforms (e.g. identity)?
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.
Ah good catch. Missed this when I ported the logic over from pyiceberg. What we should really be doing is checking if the table is partitioned and error if it is
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.
Ok checked the code and we already raise an error on a partitioned table and the method to do that is the same between pyiceberg versions. I removed this check since it won't do anything right now
PyIceberg v0.7.0 was just released. One of the new changes is the Transaction API, which replaces some of the private functions that we have been using. This PR adds support for those changes