-
-
Notifications
You must be signed in to change notification settings - Fork 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
feat(python): Make expressions containing Python UDFs serializable #18135
Conversation
CodSpeed Performance ReportMerging #18135 will not alter performanceComparing Summary
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18135 +/- ##
==========================================
+ Coverage 79.86% 79.87% +0.01%
==========================================
Files 1501 1501
Lines 202029 202069 +40
Branches 2868 2868
==========================================
+ Hits 161342 161400 +58
+ Misses 40140 40122 -18
Partials 547 547 ☔ View full report in Codecov by Sentry. |
9f310f5
to
bf444f8
Compare
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 great. Got some minor comments.
where | ||
D: Deserializer<'a>, | ||
{ | ||
use serde::de::Error; |
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.
So we could add a PythonRenamAlias
as well, I presume?
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.
Yes - we'd have to do something similar to PythonUdfExpression
where we store the lambda as a PyObject
.
But it's not super trivial and name.map
and name.map_fields
are implemented differently... I'd rather pick these up separately as I don't think those are too important and it will require some minor refactoring I think.
612671d
to
2d9704f
Compare
Changes:
Expr::AnonymousFunction
to implement SerDe for Python functions.Expr::RenameAlias
to raise an error when serializing instead of simply being skipped. Can implement SerDe for this later.