timsaucer commented on code in PR #1763:
URL:
https://github.com/apache/datafusion-python/pull/1763#discussion_r4124532378
##########
python/datafusion/functions/__init__.py:
##########
@@ -6360,16 +6808,55 @@ def nth_value(
)
-def bit_and(expression: Expr, filter: Expr | None = None) -> Expr:
+def any_value(expression: Expr, filter: Expr | None = None) -> Expr:
+ """Returns an arbitrary non-null value from each group.
+
+ Returns NULL if every value in the group is NULL. Which value is returned
+ is not specified and may differ between runs.
+
+ If using the builder functions described in ref:`_aggregation` this
function ignores
Review Comment:
Fixed in 72598193: `docs: fix the broken guide links in aggregate docstrings`
##########
python/tests/test_aggregation.py:
##########
@@ -317,11 +317,55 @@ def test_aggregate_100(df_aggregate_100, name, expr,
expected):
assert df.collect()[0].to_pydict() == expected_dict
+def test_any_value_skips_nulls_per_group():
+ ctx = SessionContext()
+ df = ctx.from_pydict(
+ {"g": ["x", "x", "y", "y", "z"], "v": [None, 7, 8, None, None]}
+ )
+ result = (
+ df.aggregate([column("g")], [f.any_value(column("v")).alias("v")])
+ .sort(column("g").sort())
+ .to_pydict()
+ )
+ assert result == {"g": ["x", "y", "z"], "v": [7, 8, None]}
+
+
[email protected](
+ ("expr", "expected"),
+ [
+ pytest.param(f.mean(column("v")), 2.0, id="mean"),
+ pytest.param(f.mean(column("v"), distinct=True), 3.0,
id="mean_distinct"),
+ pytest.param(
+ f.mean(column("v"), filter=column("v") > lit(1.0)), 5.0,
id="mean_filter"
+ ),
+ pytest.param(f.percentile_cont(column("v"), 0.5), 1.0,
id="percentile_cont"),
+ pytest.param(
+ f.percentile_cont(column("v"), 0.5, distinct=True),
+ 3.0,
+ id="percentile_cont_distinct",
+ ),
+ pytest.param(
+ f.quantile_cont(column("v"), 0.5, distinct=True),
+ 3.0,
+ id="quantile_cont_distinct",
+ ),
+ ],
+)
+def test_distinct_numeric_aggregates(expr, expected):
+ ctx = SessionContext()
+ df = ctx.from_pydict({"v": [1.0, 1.0, 1.0, 5.0]})
+ result = df.aggregate([], [expr.alias("r")]).collect_column("r")[0].as_py()
+ assert result == expected
+
+
data_test_bitwise_and_boolean_functions = [
+ ("any_value_filter", f.any_value(column("a"), filter=column("a") ==
lit(2)), [2]),
("bit_and", f.bit_and(column("a")), [0]),
("bit_and_filter", f.bit_and(column("a"), filter=column("a") != lit(2)),
[1]),
("bit_or", f.bit_or(column("b")), [6]),
("bit_or_filter", f.bit_or(column("b"), filter=column("a") != lit(3)),
[4]),
+ ("bit_and_distinct", f.bit_and(column("b"), distinct=True), [4]),
+ ("bit_or_distinct", f.bit_or(column("b"), distinct=True), [6]),
Review Comment:
Fixed in d7795029: `test: check that bit_and and bit_or keep distinct in the
expression`
##########
crates/core/src/expr.rs:
##########
@@ -743,6 +767,63 @@ impl PyExpr {
}
}
+/// Start an [`ExprFuncBuilder`] that keeps the options already set on `expr`.
+///
+/// Upstream's `ExprFunctionExt` methods on an `Expr` start from an empty
+/// builder, so `build()` would reset every option not set again. The Python
+/// function wrappers already apply their keyword options, so chaining another
+/// builder method onto their result must not discard them.
+///
+/// A built window function always stores a concrete frame, so whether the user
+/// chose it is lost. `keep_window_frame` carries that from the Python side;
when
+/// false, a frame equal to the default for the current order-by is treated as
+/// unset.
+fn builder_from_expr(expr: &Expr, keep_window_frame: bool) -> ExprFuncBuilder {
Review Comment:
Fixed in 92ace16b: `fix: raise when a builder option does not apply to the
function kind`
##########
python/datafusion/functions/__init__.py:
##########
@@ -7054,6 +7576,7 @@ def ntile(
def string_agg(
expression: Expr,
delimiter: str,
+ distinct: bool = False,
Review Comment:
Fixed in 0ac494e6: `fix: reject a non-bool distinct in string_agg`
##########
crates/core/src/functions.rs:
##########
@@ -754,16 +808,17 @@ pub fn approx_percentile_cont_with_weight(
}
#[pyfunction]
-#[pyo3(signature = (sort_expression, percentile, filter=None))]
+#[pyo3(signature = (sort_expression, percentile, distinct=None, filter=None))]
pub fn percentile_cont(
sort_expression: PySortExpr,
percentile: f64,
+ distinct: Option<bool>,
filter: Option<PyExpr>,
) -> PyDataFusionResult<PyExpr> {
let agg_fn =
functions_aggregate::expr_fn::percentile_cont(sort_expression.sort,
lit(percentile));
- add_builder_fns_to_aggregate(agg_fn, None, filter, None, None)
+ add_builder_fns_to_aggregate(agg_fn, distinct, filter, None, None)
Review Comment:
Fixed in 1b018456: `fix: keep the sort direction in percentile_cont and
friends`
##########
docs/source/user-guide/upgrade-guides.md:
##########
@@ -198,6 +198,37 @@ ctx.execute(plan, partitions=0) # before
ctx.execute(plan, partition=0) # after
```
+### More aggregate functions accept `distinct`
Review Comment:
Fixed in 6b52225a: `docs: document that chaining now keeps options already
set`
--
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]
