Copilot commented on code in PR #1783:
URL:
https://github.com/apache/datafusion-python/pull/1783#discussion_r4187456538
##########
python/datafusion/functions/__init__.py:
##########
@@ -5917,17 +5935,19 @@ def count(
"""
filter_raw = filter.expr if filter is not None else None
- if expressions is None:
+ if expressions is None or (isinstance(expressions, str) and expressions ==
"*"):
args = [Expr.literal(1).expr]
elif isinstance(expressions, list):
- args = [arg.expr for arg in expressions]
+ args = [_to_raw_expr(arg) for arg in expressions]
else:
- args = [expressions.expr]
+ args = [_to_raw_expr(expressions)]
return Expr(f.count(*args, distinct=distinct, filter=filter_raw))
Review Comment:
`count()` now more prominently supports `list[Expr | str]`, and the
docstring suggests multiple columns are valid (“a list of either”), but the PR
description notes the Rust binding only accepts a single expression (and
`count([col("a"), col("b")])` already fails on `main`). To avoid an API that
type-checks but fails at runtime, consider validating `len(expressions) <= 1`
when a list is provided and raising a targeted `TypeError` explaining the
limitation (or update the docstring/type hints to explicitly indicate only a
single-element list is supported).
##########
python/datafusion/functions/__init__.py:
##########
@@ -6142,7 +6170,7 @@ def sum(
the options ``order_by`` and ``null_treatment``.
Args:
- expression: Values to combine into an array
+ expression: Values to combine into an array (expression or column name)
Review Comment:
The `sum()` docstring argument description is incorrect (“Values to combine
into an array”); this reads like a copy/paste from `array_agg` and doesn’t
describe summation. Update it to something accurate like “Values to sum”
(keeping the new note about expression vs column name).
--
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]
