Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[FEAT] add list.value_counts() #2902

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

andrewgazelka
Copy link
Contributor

@andrewgazelka andrewgazelka commented Sep 24, 2024

still a few things to do particular with types being null or not... but at least should not crash now

daft/expressions/expressions.py Outdated Show resolved Hide resolved
src/daft-core/src/array/ops/list.rs Outdated Show resolved Hide resolved
Copy link

codspeed-hq bot commented Sep 24, 2024

CodSpeed Performance Report

Merging #2902 will not alter performance

Comparing andrew/list-value_counts (9aee66d) with main (868bbfa)

Summary

✅ 17 untouched benchmarks

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

Attention: Patch coverage is 19.80198% with 81 lines in your changes missing coverage. Please review.

Project coverage is 62.87%. Comparing base (d5b9a95) to head (99e90d0).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/daft-core/src/array/ops/list.rs 18.36% 80 Missing ⚠️
daft/expressions/expressions.py 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2902      +/-   ##
==========================================
- Coverage   66.33%   62.87%   -3.47%     
==========================================
  Files        1004     1001       -3     
  Lines      114031   116662    +2631     
==========================================
- Hits        75645    73353    -2292     
- Misses      38386    43309    +4923     
Flag Coverage Δ
62.87% <19.80%> (-3.47%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/daft-core/src/array/fixed_size_list_array.rs 79.58% <ø> (ø)
src/daft-core/src/array/list_array.rs 83.53% <ø> (ø)
src/daft-core/src/array/ops/arrow2/comparison.rs 98.70% <100.00%> (ø)
src/daft-core/src/array/struct_array.rs 79.71% <ø> (ø)
src/daft-core/src/lib.rs 100.00% <ø> (ø)
daft/expressions/expressions.py 93.25% <50.00%> (-0.12%) ⬇️
src/daft-core/src/array/ops/list.rs 82.68% <18.36%> (-11.51%) ⬇️

... and 85 files with indirect coverage changes

@andrewgazelka andrewgazelka force-pushed the andrew/list-value_counts branch 4 times, most recently from b983578 to ba181af Compare September 25, 2024 19:10
@andrewgazelka andrewgazelka changed the title value counts [FEAT] add list.value_counts() Sep 25, 2024
@github-actions github-actions bot added the enhancement New feature or request label Sep 25, 2024
@andrewgazelka andrewgazelka marked this pull request as ready for review September 25, 2024 21:13
daft/expressions/expressions.py Show resolved Hide resolved
# todo: do we want type to be a Map expression? how should we do this?
def value_counts(self) -> Expression:
"""Counts the occurrences of each unique value in the list.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Give example usage

src/arrow2/src/offset.rs Show resolved Hide resolved
src/daft-core/src/array/list_array.rs Outdated Show resolved Hide resolved
src/daft-core/src/array/ops/list.rs Show resolved Hide resolved
src/daft-core/src/array/ops/list.rs Show resolved Hide resolved
src/daft-core/src/array/ops/list.rs Show resolved Hide resolved
src/daft-core/src/array/ops/list.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants