hendrikmakait commented on code in PR #1286:
URL:
https://github.com/apache/datafusion-python/pull/1286#discussion_r2452564371
##########
python/tests/test_dataframe.py:
##########
@@ -538,15 +538,35 @@ def test_with_columns(df):
assert result.column(6) == pa.array([5, 7, 9])
-def test_with_columns_invalid_expr(df):
- with pytest.raises(TypeError, match=re.escape(EXPR_TYPE_ERROR)):
- df.with_columns("a")
- with pytest.raises(TypeError, match=re.escape(EXPR_TYPE_ERROR)):
- df.with_columns(c="a")
- with pytest.raises(TypeError, match=re.escape(EXPR_TYPE_ERROR)):
- df.with_columns(["a"])
- with pytest.raises(TypeError, match=re.escape(EXPR_TYPE_ERROR)):
- df.with_columns(c=["a"])
+def test_with_columns_str(df):
+ df = df.with_columns(
+ "a + b as c",
+ "a + b as d",
+ [
+ "a + b as e",
+ "a + b as f",
+ ],
+ g=("a + b"),
Review Comment:
What's the purpose of the parentheses?
##########
python/datafusion/dataframe.py:
##########
@@ -565,16 +566,35 @@ def with_columns(
)
Args:
- exprs: Either a single expression or an iterable of expressions to
add.
+ exprs: Either a single expression, an iterable of expressions to
add or
+ string SQL expressions.
Review Comment:
nit: Upon reading "string SQL expressions", my brain briefly thought about
string functions in SQL. Please feel free to ignore.
```suggestion
SQL expression strings.
```
##########
python/datafusion/dataframe.py:
##########
@@ -545,13 +545,14 @@ def with_column(self, name: str, expr: Expr | str) ->
DataFrame:
return DataFrame(self.df.with_column(name, ensure_expr(expr)))
def with_columns(
- self, *exprs: Expr | Iterable[Expr], **named_exprs: Expr
+ self, *exprs: Expr | str | Iterable[Expr | str], **named_exprs: Expr |
str
) -> DataFrame:
"""Add columns to the DataFrame.
- By passing expressions, iterables of expressions, or named expressions.
+ By passing expressions, iterables of expressions, string SQL
expressions,
+ or named expressions.
All expressions must be :class:`~datafusion.expr.Expr` objects created
via
- :func:`datafusion.col` or :func:`datafusion.lit`.
+ :func:`datafusion.col` or :func:`datafusion.lit` or SQL expressions.
Review Comment:
...since we define the allowed data types of expressions here.
```suggestion
:func:`datafusion.col` or :func:`datafusion.lit`, or SQL expression
strings.
```
##########
python/datafusion/dataframe.py:
##########
@@ -545,13 +545,14 @@ def with_column(self, name: str, expr: Expr | str) ->
DataFrame:
return DataFrame(self.df.with_column(name, ensure_expr(expr)))
def with_columns(
- self, *exprs: Expr | Iterable[Expr], **named_exprs: Expr
+ self, *exprs: Expr | str | Iterable[Expr | str], **named_exprs: Expr |
str
) -> DataFrame:
"""Add columns to the DataFrame.
- By passing expressions, iterables of expressions, or named expressions.
+ By passing expressions, iterables of expressions, string SQL
expressions,
+ or named expressions.
Review Comment:
I'd ignore the fact that we can use strings as expressions here, ...
```suggestion
By passing expressions, iterables of expressions, or named
expressions.
```
--
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]
