Repository: narwhals-dev/narwhals
Pull request #556 (2024-07-19 15:32:17)

Implement n_unique
<!--
# Thanks for contributing a pull request! 
## Please make sure you see our contribution guidelines: https://github.com/narwhals-dev/narwhals/blob/main/CONTRIBUTING.md
-->

## What type of PR is this? (check all applicable)

- [ ] 💾 Refactor
- [x] ✨ Feature
- [ ] 🐛 Bug Fix
- [ ] 🔧 Optimization
- [ ] 📝 Documentation
- [ ] ✅ Test
- [ ] 🐳 Other

## Related issues 

- Related issue #372
- Closes #

## Checklist

- [ x] Code follows style guide (ruff)
- [ ] Tests added 
- [ ] Documented the changes

## If you have comments or can explain your changes, please do so below.
I implemented `n_unique`. A test file already existed, so I did not have to write a test


MarcoGorelli: thanks @mistShard !

It's true that a test file exists, but it does not yet test pyarrow

could you remove the lines
```
    if "pyarrow_table" in str(constructor):
        request.applymarker(pytest.mark.xfail)
```
from the test, and check that it still passes?
mistShard: When I deleted the lines you requested, the test threw the following error:

```
========================================================== test session starts ===========================================================
platform win32 -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0
rootdir: C:\Users\hp\Documents\Work\OpenTeams\narwhals
configfile: pyproject.toml
plugins: hypothesis-6.103.2, cov-5.0.0
collected 5 items

tests\expr_and_series\n_unique_test.py ....F                                                                                        [100%]

================================================================ FAILURES ================================================================ 
________________________________________________ test_n_unique[pyarrow_table_constructor] ________________________________________________ 

request = <FixtureRequest for <Function test_n_unique[pyarrow_table_constructor]>>
constructor = <function pyarrow_table_constructor at 0x000002089171E7A0>

    def test_n_unique(request: Any, constructor: Any) -> None:

        df = nw.from_native(constructor(data), eager_only=True)
        result = df.select(nw.all().n_unique())
        expected = {
            "a": [3],
            "b": [4],
        }
>       compare_dicts(result, expected)

tests\expr_and_series\n_unique_test.py:22:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

result = ┌───────────────────────────────────────────────┐
| Narwhals DataFrame                            |
| Use `narwhals.to_native` to see native output |
└───────────────────────────────────────────────┘
expected = {'a': [3], 'b': [4]}

    def compare_dicts(result: Any, expected: dict[str, Any]) -> None:
        if hasattr(result, "collect"):
            result = result.collect()
        if hasattr(result, "columns"):
            for key in result.columns:
                assert key in expected
        for key in expected:
            for lhs, rhs in zip_strict(result[key], expected[key]):
                if hasattr(lhs, "as_py"):
                    lhs = lhs.as_py()  # noqa: PLW2901
                if hasattr(rhs, "as_py"):  # pragma: no cover
                    rhs = rhs.as_py()  # noqa: PLW2901
                if isinstance(lhs, float) and not math.isnan(lhs):
                    assert math.isclose(lhs, rhs, rel_tol=0, abs_tol=1e-6), (lhs, rhs)
                elif isinstance(lhs, float) and math.isnan(lhs):
                    assert math.isnan(rhs), (lhs, rhs)  # pragma: no cover
                elif pd.isna(lhs):
                    assert pd.isna(rhs), (lhs, rhs)
                else:
>                   assert lhs == rhs, (lhs, rhs)
E                   AssertionError: (2, 3)

tests\utils.py:39: AssertionError
======================================================== short test summary info =========================================================
FAILED tests/expr_and_series/n_unique_test.py::test_n_unique[pyarrow_table_constructor] - AssertionError: (2, 3)
====================================================== 1 failed, 4 passed in 1.61s ======================================================= 
```
MarcoGorelli: thanks @mistShard ! that's a clue that something needs changing in the implementation 😉 
mistShard: @MarcoGorelli I'll resolve it. A preliminary examination suggests that it has to do with `None`s not being counted by PyArrow's `.count()`
MarcoGorelli: that's a good observation! does it work to just use `len`?
mistShard: @MarcoGorelli Thanks for the feedback. I found that `pyarrow.compute` does not have a `len` nmethod (_maybe I just didn't see it and it was staring me in the face_). Instead I used the `mode="all"` parameter to make the `.count()` method operate on all unique values null or otherwise.

`n_unique` passed the test after I made the alterations I did.
MarcoGorelli: woooow! well done @mistShard !

I think the mypy issue is unrelated, will, but this should be it! nice one!
MarcoGorelli: ah yeah, there's been a new mypy release

if you mark as ready for review, we can ship this one!
mistShard: @MarcoGorelli marked for review
MarcoGorelli: 😄 let's ship it - seriously great job here @mistShard , it's hard to believe that just a few weeks ago you weren't familiar with pandas and now you've figured this all out by yourself

Patch:
diff --git a/narwhals/_arrow/expr.py b/narwhals/_arrow/expr.py
index f8bfbffc..d3e9ab05 100644
--- a/narwhals/_arrow/expr.py
+++ b/narwhals/_arrow/expr.py
@@ -144,6 +144,9 @@ class ArrowExpr:
     def count(self) -> Self:
         return reuse_series_implementation(self, "count", returns_scalar=True)
 
+    def n_unique(self) -> Self:
+        return reuse_series_implementation(self, "n_unique", returns_scalar=True)
+
     def std(self, ddof: int = 1) -> Self:
         return reuse_series_implementation(self, "std", ddof=ddof, returns_scalar=True)
 
diff --git a/narwhals/_arrow/series.py b/narwhals/_arrow/series.py
index c287dd81..3e87d9ef 100644
--- a/narwhals/_arrow/series.py
+++ b/narwhals/_arrow/series.py
@@ -175,6 +175,12 @@ class ArrowSeries:
         pc = get_pyarrow_compute()
         return pc.count(self._native_series)  # type: ignore[no-any-return]
 
+    def n_unique(self) -> int:
+        pc = get_pyarrow_compute()
+        unique_values = pc.unique(self._native_series)
+        count_unique = pc.count(unique_values, mode="all")
+        return count_unique.as_py()  # type: ignore[no-any-return]
+
     def __narwhals_namespace__(self) -> ArrowNamespace:
         from narwhals._arrow.namespace import ArrowNamespace
 
diff --git a/utils/check_backend_completeness.py b/utils/check_backend_completeness.py
index 90506dc4..c3e06858 100644
--- a/utils/check_backend_completeness.py
+++ b/utils/check_backend_completeness.py
@@ -26,7 +26,6 @@ MISSING = [
     "Series.is_last_distinct",
     "Series.is_sorted",
     "Series.is_unique",
-    "Series.n_unique",
     "Series.quantile",
     "Series.round",
     "Series.shift",


Test patch:
diff --git a/tests/expr_and_series/n_unique_test.py b/tests/expr_and_series/n_unique_test.py
index 291a0a43..98dd0a85 100644
--- a/tests/expr_and_series/n_unique_test.py
+++ b/tests/expr_and_series/n_unique_test.py
@@ -1,7 +1,5 @@
 from typing import Any
 
-import pytest
-
 import narwhals.stable.v1 as nw
 from tests.utils import compare_dicts
 
@@ -11,10 +9,7 @@ data = {
 }
 
 
-def test_n_unique(request: Any, constructor: Any) -> None:
-    if "pyarrow_table" in str(constructor):
-        request.applymarker(pytest.mark.xfail)
-
+def test_n_unique(constructor: Any) -> None:
     df = nw.from_native(constructor(data), eager_only=True)
     result = df.select(nw.all().n_unique())
     expected = {