Repository: narwhals-dev/narwhals
Pull request #495 (2024-07-11 21:10:32)

feat: Series.sum test case
<!--
# 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
- [x] ✅ Test
- [ ] 🐳 Other

## Related issues 

- Related issue # 
- Closes #

## Checklist

- [ ] Code follows style guide (ruff)
- [x] Tests added 
- [ ] Documented the changes

## If you have comments or can explain your changes, please do so below.



MarcoGorelli: hey - you're missing quite a few parts, this should be very similar to https://github.com/narwhals-dev/narwhals/pull/457/files, please check that
MarcoGorelli: hi @mfonekpo - is it clear what you should do here?
mfonekpo: Yes Marco, I looked through the example you gave last night. I will
implement them today.

On Fri, 12 Jul 2024, 15:51 Marco Edward Gorelli, ***@***.***>
wrote:

> hi @mfonekpo <https://github.com/mfonekpo> - is it clear what you should
> do here?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/narwhals-dev/narwhals/pull/495#issuecomment-2225759602>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AOA4GJBERQMIOCJOIAG5SVTZL7UO3AVCNFSM6AAAAABKXXSZG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRVG42TSNRQGI>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

MarcoGorelli: thanks @mfonekpo for updating

could you take another look at https://github.com/narwhals-dev/narwhals/pull/457/files please? it should be identical to that PR, but with `sum` instead of `min` / `max`
MarcoGorelli: almost there, just need to address
```
tests/expr/sum_all_test.py:7:18: ARG001 Unused function argument: `request`
  |
7 | def test_sum_all(request: Any, constructor: Any) -> None:
  |                  ^^^^^^^ ARG001
8 |     data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8, 9]}
9 |     df = nw.from_native(constructor(data), eager_only=True)
  |

tests/stable_api_test.py:11:31: ARG001 Unused function argument: `request`
   |
11 | def test_renamed_taxicab_norm(request: Any, constructor: Any) -> None:
   |                               ^^^^^^^ ARG001
12 |     # Suppose we need to rename `_l1_norm` to `_taxicab_norm`.
13 |     # We need `narwhals.stable.v1` to stay stable. So, we
   |
```
please make sure that `pre-commit run --all-files` passes - almost there!

Patch:
diff --git a/narwhals/_arrow/expr.py b/narwhals/_arrow/expr.py
index eea0c6cc..9635b835 100644
--- a/narwhals/_arrow/expr.py
+++ b/narwhals/_arrow/expr.py
@@ -165,6 +165,9 @@ class ArrowExpr:
     def all(self) -> Self:
         return reuse_series_implementation(self, "all", returns_scalar=True)
 
+    def sum(self) -> Self:
+        return reuse_series_implementation(self, "sum", returns_scalar=True)
+
     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 af9fc7c0..5d1ae9a9 100644
--- a/narwhals/_arrow/series.py
+++ b/narwhals/_arrow/series.py
@@ -155,6 +155,10 @@ class ArrowSeries:
         pc = get_pyarrow_compute()
         return pc.max(self._native_series)  # type: ignore[no-any-return]
 
+    def sum(self) -> int:
+        pc = get_pyarrow_compute()
+        return pc.sum(self._native_series)  # type: ignore[no-any-return]
+
     def std(self, ddof: int = 1) -> int:
         pc = get_pyarrow_compute()
         return pc.stddev(self._native_series, ddof=ddof)  # type: ignore[no-any-return]
diff --git a/utils/check_backend_completeness.py b/utils/check_backend_completeness.py
index a9b6ae73..ab2eec28 100644
--- a/utils/check_backend_completeness.py
+++ b/utils/check_backend_completeness.py
@@ -35,7 +35,6 @@ MISSING = [
     "Series.sample",
     "Series.shift",
     "Series.sort",
-    "Series.sum",
     "Series.to_frame",
     "Series.to_pandas",
     "Series.unique",


Test patch:
diff --git a/tests/expr/sum_all_test.py b/tests/expr/sum_all_test.py
index cf2103f8..2fbfb625 100644
--- a/tests/expr/sum_all_test.py
+++ b/tests/expr/sum_all_test.py
@@ -1,15 +1,10 @@
 from typing import Any
 
-import pytest
-
 import narwhals.stable.v1 as nw
 from tests.utils import compare_dicts
 
 
-def test_sum_all(request: Any, constructor: Any) -> None:
-    if "pyarrow_table" in str(constructor):
-        request.applymarker(pytest.mark.xfail)
-
+def test_sum_all(constructor: Any) -> None:
     data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8, 9]}
     df = nw.from_native(constructor(data), eager_only=True)
     result = df.select(nw.all().sum())
diff --git a/tests/frame/series_sum_test.py b/tests/frame/series_sum_test.py
new file mode 100644
index 00000000..3a1c1d95
--- /dev/null
+++ b/tests/frame/series_sum_test.py
@@ -0,0 +1,23 @@
+from __future__ import annotations
+
+from typing import Any
+
+import narwhals.stable.v1 as nw
+from tests.utils import compare_dicts
+
+
+def test_series_sum(constructor: Any) -> None:
+    data = {
+        "a": [0, 1, 2, 3, 4],
+        "b": [1, 2, 3, 5, 3],
+        "c": [5, 4, None, 2, 1],
+    }
+    df = nw.from_native(
+        constructor(data), strict=False, eager_only=True, allow_series=True
+    )
+
+    result = df.select(nw.col("a", "b", "c").sum())
+
+    expected_sum = {"a": [10], "b": [14], "c": [12]}
+
+    compare_dicts(result, expected_sum)
diff --git a/tests/stable_api_test.py b/tests/stable_api_test.py
index 205eebee..211ba865 100644
--- a/tests/stable_api_test.py
+++ b/tests/stable_api_test.py
@@ -8,7 +8,7 @@ import narwhals.stable.v1 as nw_v1
 from tests.utils import compare_dicts
 
 
-def test_renamed_taxicab_norm(request: Any, constructor: Any) -> None:
+def test_renamed_taxicab_norm(constructor: Any) -> None:
     # Suppose we need to rename `_l1_norm` to `_taxicab_norm`.
     # We need `narwhals.stable.v1` to stay stable. So, we
     # make the change in `narwhals`, and then add the new method
@@ -17,9 +17,6 @@ def test_renamed_taxicab_norm(request: Any, constructor: Any) -> None:
     # API will still be able to use it, without the main namespace
     # getting cluttered by the new name.
 
-    if "pyarrow_table" in str(constructor):
-        request.applymarker(pytest.mark.xfail)
-
     df = nw.from_native(constructor({"a": [1, 2, 3, -4, 5]}))
     result = df.with_columns(b=nw.col("a")._taxicab_norm())
     expected = {"a": [1, 2, 3, -4, 5], "b": [15] * 5}