Michael-J-Ward commented on code in PR #771:
URL: https://github.com/apache/datafusion-python/pull/771#discussion_r1693037454
##########
src/functions.rs:
##########
@@ -30,13 +31,147 @@ use datafusion::functions_aggregate;
use datafusion_common::{Column, ScalarValue, TableReference};
use datafusion_expr::expr::Alias;
use datafusion_expr::{
- aggregate_function,
expr::{
find_df_window_func, AggregateFunction, AggregateFunctionDefinition,
Sort, WindowFunction,
},
lit, Expr, WindowFunctionDefinition,
};
+#[pyfunction]
+pub fn approx_distinct(expression: PyExpr) -> PyExpr {
+
functions_aggregate::expr_fn::approx_distinct::approx_distinct(expression.expr).into()
+}
+
+#[pyfunction]
+pub fn approx_median(expression: PyExpr, distinct: bool) -> PyResult<PyExpr> {
+ let expr = functions_aggregate::expr_fn::approx_median(expression.expr);
+ if distinct {
+ Ok(expr.distinct().build()?.into())
+ } else {
+ Ok(expr.into())
+ }
+}
Review Comment:
I agree.
One of the first functions I went to port was `first_value`, and I came away
certain that we'd end up some Builder API around the aggregate / window
functions. Additionally, the `functions` currently expose different builder
args.
So, I decided to go the explicit route instead of writing a `macro_rules!`
just to change it next release
(That's why I was excited to see you doing precisesly that `Builder` API
work upstream.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
