Skip to content

Convert Correlation to UDAF #10884

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
jayzhan211 opened this issue Jun 12, 2024 · 1 comment · Fixed by #11064
Closed

Convert Correlation to UDAF #10884

jayzhan211 opened this issue Jun 12, 2024 · 1 comment · Fixed by #11064
Assignees
@pingsutw
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jayzhan211
Copy link
Contributor

jayzhan211 commented Jun 12, 2024 •
edited
Loading

Is your feature request related to a problem or challenge?

Similar to other issues in #8708 Move datafusion/physical-expr/src/aggregate/correlation.rs to datafusion/functions-aggregate/src/correlation.rs

I found CovarianceAccumulator in datafusion/physical-expr/src/aggregate/covariance.rs is not removed, we should remove it and use the one in datafusion/functions-aggregate/src/covariance.rs instead. The same with StddevAccumulator

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response
@jayzhan211 jayzhan211 added the enhancement New feature or request label Jun 12, 2024
@jayzhan211 jayzhan211 mentioned this issue Jun 12, 2024
Closed
@jayzhan211 jayzhan211 added the good first issue Good for newcomers label Jun 12, 2024
@pingsutw
Copy link
Member

take

@pingsutw pingsutw mentioned this issue Jun 22, 2024
Merged
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this issue Jul 24, 2024
@Michael-J-Ward
migrage corr to UDAF
da69cee 
Ref: apache/datafusion#10884
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this issue Jul 24, 2024
@Michael-J-Ward
migrage corr to UDAF
70069d7 
Ref: apache/datafusion#10884
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this issue Jul 25, 2024
@Michael-J-Ward
migrage corr to UDAF
98498e9 
Ref: apache/datafusion#10884
andygrove pushed a commit to apache/datafusion-python that referenced this issue Jul 31, 2024
@Michael-J-Ward
Upgrade Datafusion 40 (#771)
f580155 
* chore: update datafusion deps

* feat: impl ExecutionPlan::static_name() for DatasetExec

This required trait method was added upstream [0] and recommends to simply forward to `static_name`.

[0]: apache/datafusion#10266

* feat: update first_value and last_value wrappers.

Upstream signatures were changed for the new new `AggregateBuilder` api [0].

This simply gets the code to work. We should better incorporate that API into `datafusion-python`.

[0] apache/datafusion#10560

* migrate count to UDAF

Builtin Count was removed upstream.

TBD whether we want to re-implement `count_star` with new API.

Ref: apache/datafusion#10893

* migrate approx_percentile_cont, approx_distinct, and approx_median to UDAF

Ref: approx_distinct apache/datafusion#10851
Ref: approx_median apache/datafusion#10840
Ref: approx_percentile_cont and _with_weight apache/datafusion#10917

* migrate avg to UDAF

Ref: apache/datafusion#10964

* migrage corr to UDAF

Ref: apache/datafusion#10884

* migrate grouping to UDAF

Ref: apache/datafusion#10906

* add alias `mean` for UDAF `avg`

* migrate stddev to UDAF

Ref: apache/datafusion#10827

* remove rust alias for stddev

The python wrapper now provides stddev_samp alias.

* migrage var_pop to UDAF

Ref: apache/datafusion#10836

* migrate regr_* functions to UDAF

Ref: apache/datafusion#10898

* migrate bitwise functions to UDAF

The functions now take a single expression instead of a Vec<_>.

Ref: apache/datafusion#10930

* add missing variants for ScalarValue with todo

* fix typo in approx_percentile_cont

* add distinct arg to count

* comment out failing test

`approx_percentile_cont` is now returning a DoubleArray instead of an IntArray.

This may be a bug upstream; it requires further investigation.

* update tests to expect lowercase `sum` in query plans

This was changed upstream.

Ref: apache/datafusion#10831

* update ScalarType data_type map

* add docs dependency pickleshare

* re-implement count_star

* lint: ruff python lint

* lint: rust cargo fmt

* include name of window function in error for find_window_fn

* refactor `find_window_fn` for debug clarity

* search default aggregate functions by both name and aliases

The alias list no longer includes the name of the function.

Ref: apache/datafusion#10658

* fix markdown in find_window_fn docs

* parameterize test_window_functions

`first_value` and `last_value` are currently failing and marked as xfail.

* add test ids to test_simple_select tests marked xfail

* update find_window_fn to search built-ins first

The behavior of `first_value` and `last_value` UDAFs currently does not match the built-in behavior.
This allowed me to remove `marks=pytest.xfail` from the window tests.

* improve first_call and last_call use of the builder API

* remove trailing todos

* fix examples/substrait.py

* chore: remove explicit aliases from functions.rs

Ref: #779

* remove `array_fn!` aliases

* remove alias rules for `expr_fn_vec!`

* remove alias rules from `expr_fn!` macro

* remove unnecessary pyo3 var-arg signatures in functions.rs

* remove pyo3 signatures that provided defaults for first_value and last_value

* parametrize test_string_functions

* test regr_ function wrappers

Closes #778
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@pingsutw pingsutw

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Convert Correlation to UDAF pingsutw/datafusion
2 participants
@jayzhan211 @pingsutw