Row UDF scalar arguments #311
Conversation
|
This is ready for review - I thought some code might need to be written to disambiguate when one has a column with a name that could also be a constant, like having a column with name |
There was a problem hiding this comment.
Thanks a lot for the changes @brandon-b-miller !
Couple of minor comments in addition to the changes suggested:
Would it be possible to also update the docstring to include an example with scalar values?
Also I was able to catch the issues below while testing this with udfs that accept multiple column args, since in the single arg case the output would be correct. Might be worth adding that case to the tests as well.
dask_sql/datacontainer.py
Outdated
| scalar_args.append(operand) | ||
| df = column_args[0].to_frame() | ||
| for col in column_args[1:]: | ||
| df[col.name] = operand |
There was a problem hiding this comment.
| df[col.name] = operand | |
| df[col.name] = col |
dask_sql/datacontainer.py
Outdated
| df = column_args[0].to_frame() | ||
| for col in column_args[1:]: | ||
| df[col.name] = operand |
There was a problem hiding this comment.
I'm guessing this section was meant to be outside the loop.
|
Updated! :) |
There was a problem hiding this comment.
Thanks for the work here @brandon-b-miller 🙂 just small nitpicks, the code itself LGTM
tests/integration/test_function.py
Outdated
| assert_frame_equal(return_df.reset_index(drop=True), expectation) | ||
|
|
||
|
|
||
| # Test row UDFs with one args |
There was a problem hiding this comment.
| # Test row UDFs with one args | |
| # Test row UDFs with two args |
tests/integration/test_function.py
Outdated
| @@ -63,6 +65,64 @@ def f(row): | |||
| assert_frame_equal(return_df.reset_index(drop=True), expectation) | |||
|
|
|||
|
|
|||
| # Test row UDFs with one args | |||
There was a problem hiding this comment.
| # Test row UDFs with one args | |
| # Test row UDFs with one arg |
dask_sql/datacontainer.py
Outdated
| column_args.append(operand) | ||
| else: | ||
| scalar_args.append(operand) | ||
| df = column_args[0].to_frame() |
There was a problem hiding this comment.
| df = column_args[0].to_frame() | |
| df = column_args[0].to_frame() |
Codecov Report
@@ Coverage Diff @@
## main #311 +/- ##
==========================================
- Coverage 95.75% 95.69% -0.06%
==========================================
Files 65 65
Lines 2802 2810 +8
Branches 418 421 +3
==========================================
+ Hits 2683 2689 +6
- Misses 77 78 +1
- Partials 42 43 +1
Continue to review full report at Codecov.
|
Closes #279