Skip to content

Commit

Permalink
refactor: make sure value_counts always has a projection
Browse files Browse the repository at this point in the history
  • Loading branch information
cpcloud authored and kszucs committed Dec 6, 2022
1 parent 0234e5c commit a70a302
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
11 changes: 7 additions & 4 deletions ibis/expr/types/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -729,10 +729,13 @@ def value_counts(self, metric_name: str = "count") -> ir.Table:
"""
from ibis.expr.analysis import find_first_base_table

base = find_first_base_table(self.op()).to_expr()
metric = base.count().name(metric_name)

return base.group_by(self).aggregate(metric)
return (
find_first_base_table(self.op())
.to_expr()
.select(self)
.group_by(self.get_name())
.agg(**{metric_name: lambda t: t.count()})
)

def first(self) -> Column:
return ops.FirstValue(self).to_expr()
Expand Down
2 changes: 1 addition & 1 deletion ibis/tests/expr/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def test_group_by_column_select_api(table):
def test_value_counts_convenience(table):
# #152
result = table.g.value_counts()
expected = table.group_by('g').aggregate(table.count().name('count'))
expected = table.select('g').group_by('g').aggregate(count=lambda t: t.count())

assert_equal(result, expected)

Expand Down

0 comments on commit a70a302

Please sign in to comment.