Repository: narwhals-dev/narwhals
Pull request #576 (2024-07-22 10:00:04)

feat: add arrow value_counts 

## What type of PR is this? (check all applicable)

- [ ] 💾 Refactor
- [x] ✨ Feature
- [ ] 🐛 Bug Fix
- [ ] 🔧 Optimization
- [ ] 📝 Documentation
- [x] ✅ Test
- [ ] 🐳 Other

## Related issues #

- Related issue #372 
- Closes #

## Checklist

- [x] Code follows style guide (ruff)
- [x] Tests added 
- [ ] Documented the changes

## If you have comments or can explain your changes, please do so below.




Patch:
diff --git a/narwhals/_arrow/expr.py b/narwhals/_arrow/expr.py
index 0f0d5c3c..d0082a31 100644
--- a/narwhals/_arrow/expr.py
+++ b/narwhals/_arrow/expr.py
@@ -189,6 +189,9 @@ class ArrowExpr:
     def sum(self) -> Self:
         return reuse_series_implementation(self, "sum", returns_scalar=True)
 
+    def drop_nulls(self) -> Self:
+        return reuse_series_implementation(self, "drop_nulls")
+
     def alias(self, name: str) -> Self:
         # Define this one manually, so that we can
         # override `output_names` and not increase depth
diff --git a/narwhals/_arrow/series.py b/narwhals/_arrow/series.py
index f6828a30..b51ca401 100644
--- a/narwhals/_arrow/series.py
+++ b/narwhals/_arrow/series.py
@@ -203,6 +203,10 @@ class ArrowSeries:
         pc = get_pyarrow_compute()
         return pc.sum(self._native_series)  # type: ignore[no-any-return]
 
+    def drop_nulls(self) -> ArrowSeries:
+        pc = get_pyarrow_compute()
+        return self._from_native_series(pc.drop_null(self._native_series))
+
     def std(self, ddof: int = 1) -> int:
         pc = get_pyarrow_compute()
         return pc.stddev(self._native_series, ddof=ddof)  # type: ignore[no-any-return]
@@ -340,6 +344,30 @@ class ArrowSeries:
             return self._native_series[0].as_py()
         return self._native_series[index].as_py()
 
+    def value_counts(self: Self, *, sort: bool = False, parallel: bool = False) -> Any:  # noqa: ARG002
+        """Parallel is unused, exists for compatibility"""
+        from narwhals._arrow.dataframe import ArrowDataFrame
+
+        pc = get_pyarrow_compute()
+        pa = get_pyarrow()
+
+        name_ = (
+            "index" if self._native_series._name is None else self._native_series._name
+        )
+
+        val_count = pc.value_counts(self._native_series)
+        val_count = pa.Table.from_arrays(
+            [val_count.field("values"), val_count.field("counts")], names=[name_, "count"]
+        )
+
+        if sort:
+            val_count = val_count.sort_by([("count", "descending")])
+
+        return ArrowDataFrame(
+            val_count,
+            backend_version=self._backend_version,
+        )
+
     def zip_with(self: Self, mask: Self, other: Self) -> Self:
         pc = get_pyarrow_compute()
 
diff --git a/narwhals/_pandas_like/series.py b/narwhals/_pandas_like/series.py
index 650225af..de85df4e 100644
--- a/narwhals/_pandas_like/series.py
+++ b/narwhals/_pandas_like/series.py
@@ -567,7 +567,7 @@ class PandasLikeSeries:
         ).reset_index()
         val_count.columns = [name_, "count"]
         if sort:
-            val_count = val_count.sort_values(name_)
+            val_count = val_count.sort_values("count", ascending=False)
 
         return PandasLikeDataFrame(
             val_count,
diff --git a/utils/check_backend_completeness.py b/utils/check_backend_completeness.py
index b83766bc..ad486f4b 100644
--- a/utils/check_backend_completeness.py
+++ b/utils/check_backend_completeness.py
@@ -14,7 +14,6 @@ from narwhals._arrow.dataframe import ArrowDataFrame
 MISSING = [
     "DataFrame.iter_rows",
     "DataFrame.pipe",
-    "Series.drop_nulls",
     "Series.from_iterable",
     "Series.is_between",
     "Series.quantile",
@@ -22,7 +21,6 @@ MISSING = [
     "Series.shift",
     "Series.sort",
     "Series.unique",
-    "Series.value_counts",
 ]
 
 


Test patch:
diff --git a/tests/expr_and_series/drop_nulls_test.py b/tests/expr_and_series/drop_nulls_test.py
new file mode 100644
index 00000000..ef109e01
--- /dev/null
+++ b/tests/expr_and_series/drop_nulls_test.py
@@ -0,0 +1,31 @@
+from __future__ import annotations
+
+from typing import Any
+
+import narwhals as nw
+from tests.utils import compare_dicts
+
+
+def test_drop_nulls(constructor: Any) -> None:
+    data = {
+        "A": [1, 2, None, 4],
+        "B": [5, 6, 7, 8],
+        "C": [None, None, None, None],
+        "D": [9, 10, 11, 12],
+    }
+
+    df = nw.from_native(constructor(data), eager_only=True)
+
+    result_a = df.select(nw.col("A").drop_nulls())
+    result_b = df.select(df["B"].drop_nulls())
+    result_c = df.select(nw.col("C").drop_nulls())
+    result_d = df.select(nw.col("D").drop_nulls())
+    expected_a = {"A": [1.0, 2.0, 4.0]}
+    expected_b = {"B": [5, 6, 7, 8]}
+    expected_c = {"C": []}  # type: ignore[var-annotated]
+    expected_d = {"D": [9, 10, 11, 12]}
+
+    compare_dicts(result_a, expected_a)
+    compare_dicts(result_b, expected_b)
+    compare_dicts(result_c, expected_c)
+    compare_dicts(result_d, expected_d)
diff --git a/tests/frame/test_common.py b/tests/frame/test_common.py
index bf7f619f..9030f99c 100644
--- a/tests/frame/test_common.py
+++ b/tests/frame/test_common.py
@@ -159,21 +159,6 @@ def test_expr_na(constructor_with_lazy: Any) -> None:
     compare_dicts(result_nna, expected)
 
 
-def test_drop_nulls(request: Any, constructor_with_lazy: Any) -> None:
-    if "pyarrow_table" in str(constructor_with_lazy):
-        request.applymarker(pytest.mark.xfail)
-
-    df = nw.from_native(constructor_with_lazy(data_na)).lazy()
-
-    result = df.select(nw.col("a").drop_nulls())
-    expected = {"a": [3, 2]}
-    compare_dicts(result, expected)
-
-    result = df.select(df.collect()["a"].drop_nulls())
-    expected = {"a": [3, 2]}
-    compare_dicts(result, expected)
-
-
 @pytest.mark.parametrize(
     ("drop", "left"),
     [
diff --git a/tests/series_only/test_common.py b/tests/series_only/test_common.py
index bb5996e9..499bd652 100644
--- a/tests/series_only/test_common.py
+++ b/tests/series_only/test_common.py
@@ -114,29 +114,6 @@ def test_to_numpy() -> None:
     assert nw_series.shape == (3,)
 
 
-def test_value_counts(request: Any, constructor_series: Any) -> None:
-    if "pyarrow_series" in str(constructor_series):
-        request.applymarker(pytest.mark.xfail)
-
-    if "pandas_series_nullable" in str(constructor_series):  # fails for py3.8
-        pytest.skip()
-
-    series = nw.from_native(constructor_series(data_dups).rename("b"), series_only=True)
-
-    sorted_result = series.value_counts(sort=True)
-    assert sorted_result.columns == ["b", "count"]
-
-    expected = np.array([[4, 2], [6, 1]])
-    assert (sorted_result.to_numpy() == expected).all()
-
-    unsorted_result = series.value_counts(sort=False)
-    assert unsorted_result.columns == ["b", "count"]
-
-    a = unsorted_result.to_numpy()
-
-    assert (a[a[:, 0].argsort()] == expected).all()
-
-
 @pytest.mark.parametrize(
     ("interpolation", "expected"),
     [
diff --git a/tests/series_only/value_counts_test.py b/tests/series_only/value_counts_test.py
new file mode 100644
index 00000000..abda5979
--- /dev/null
+++ b/tests/series_only/value_counts_test.py
@@ -0,0 +1,26 @@
+import sys
+from typing import Any
+
+import pytest
+
+import narwhals.stable.v1 as nw
+from tests.utils import compare_dicts
+
+data = {"a": [4, 4, 6, 4, 1, 1]}
+
+
+def test_value_counts(request: Any, constructor: Any) -> None:
+    if "pandas_nullable_constructor" in str(constructor) and sys.version_info < (
+        3,
+        9,
+    ):  # fails for py3.8
+        request.applymarker(pytest.mark.xfail)
+
+    series = nw.from_native(constructor(data), eager_only=True)["a"]
+
+    sorted_result = series.value_counts(sort=True)
+    expected = {"a": [4, 1, 6], "count": [3, 2, 1]}
+    compare_dicts(sorted_result, expected)
+
+    unsorted_result = series.value_counts(sort=False).sort("count", descending=True)
+    compare_dicts(unsorted_result, expected)