feat: Add `assert_series_equal` #2983

FBruzzesi · 2025-08-13T18:45:52Z

Description

This PR introduces the testing module (mirroring the polars structure) and assert_series_equal. It's a step towards: #739 and #2804.

I think it's a good structure to leave space for other modules (let's say, a constructors module within it, see #2959 and in particular the discord conversation linked in the issue).

Additional comments:

One edge case/blocker for nested dtype here is mentioned in [Enh]: Sorting complex types #2939 (I am raising a NotImplementedError).
I started with assert_series_equal since if this goes through, then assert_frame_equal can re-use it for the value checks. However I didn't want to bloat the PR with 1k+ lines changes
As usual... 600+ lines changes, but in all honesty 50%+ are tests.
If this and assert_frame_equal end up being implemented, I think we can have an issue to track usage for them in the test suite itself

What type of PR is this? (check all applicable)

Checklist

Code follows style guide (ruff)
Tests added
Documented the changes

If you have comments or can explain your changes, please do so below

FBruzzesi · 2025-08-13T21:08:26Z

modin & cudf `clip` issue leading to `is_close` exception

The modin errors are related to `is_close` failing, which is because of `clip` with pyarrow integers (I couldn't really replicate with floats). And that brought up an old memory: https://github.com/modin-project/modin/issues/7415.

Which now leads to the harsh part:

It's my fault that we didn't catch that in is_close
fix: Workaround for clip with modin[pyarrow] and cudf backends #2986 should fix it

Once that's merged, I will take care of the issues in the tests with old pandas version

is now solved with a workaround which enables clip to work without issues on both cudf and modin (#2986)

dangotbanned · 2025-08-28T20:41:06Z

`rust` impl

Just saving this here as I've thought of and forgotten to find it 3 times now 😂

https://github.com/pola-rs/polars/blob/cbf621add84d0a4d51b361b25132f61b61ff9126/crates/polars-testing/src/asserts/utils.rs

dangotbanned · 2025-08-29T21:48:12Z

narwhals/testing/asserts/series.py

+def assert_series_equal(
+    left: IntoSeriesT,
+    right: IntoSeriesT,
+    *,
+    check_dtypes: bool = True,
+    check_names: bool = True,
+    check_order: bool = True,
+    check_exact: bool = False,
+    rel_tol: float = 1e-05,
+    abs_tol: float = 1e-08,
+    categorical_as_str: bool = False,
+) -> None:
+    """Assert that the left and right Series are equal.
+
+    Raises a detailed `AssertionError` if the Series differ.
+    This function is intended for use in unit tests.
+
+    Arguments:
+        left: The first Series to compare.
+        right: The second Series to compare.


I don't think we should be accepting anything that isn't nw.Series here

I'm not opposed to that being another function (e.g. assert_native_series_equal), but this one should just be comparing - not doing conversion as well IMO

That's fair (and thank for the commits fixing it). However, then we might need to be even more e.g. also check that the narwhals namespace version is the same. In principle the check:

narwhals/narwhals/testing/asserts/series.py

Lines 83 to 90 in 5d7040b

if any(not is_narwhals_series(obj) for obj in (left, right)):

msg = (

"Expected `narwhals.Series` instance, found:\n"

f"[left]: {qualified_type_name(type(left))}\n"

f"[right]: {qualified_type_name(type(left))}\n\n"

"Hint: Use `nw.from_native(obj, series_only=True) to convert to `narwhals.Series`"

)

raise TypeError(msg)

would pass for:

import narwhals.stable.v1 as nw_v1 import narwhals.stable.v2 as nw_v2 assert_series_equal( nw_v1.from_native(...), nw_v2.from_native(...), ... )

#2983 (comment)

Ah I may have been too quick suggesting this 😅

Coercing the right or whatever is equivalent to expected; might make sense for ergonomics. E.g. like our assert_equal_data

But I think allowing both inputs to be native is a bit much

I'm guessing you've already checked this, but pandas has quite a few of these

https://github.com/pandas-dev/pandas/blob/7bfef3b1cba58a6c3aa62493e0b0905bc59e6443/pandas/_testing/asserters.py

But I think allowing both inputs to be native is a bit much

I might disagree with this. I would prefer having the API to be symmetric in both left and right arguments, so either cast both or none of them. We can have both assert_series_equal and assert_native_series_equal as you suggested. The second is quite low effort after the first

Coercing the right or whatever is equivalent to expected; might make sense for ergonomics. E.g. like our assert_equal_data

We can always create a tests utility function for better ergonomics!

#2983 (comment)

I actually didn't check pandas at all 🙈

I might disagree with this. I would prefer having the API to be symmetric in both left and right arguments, so either cast both or none of them.

Totally fair!

I actually didn't check pandas at all 🙈

Now that surprised me!

I suspected the _check_* naming scheme and usage of __tracebackhide__ came from there - but just an interesting coincidence it seems 😂

#2983 (comment)

dangotbanned · 2025-08-29T22:26:03Z

narwhals/testing/asserts/series.py

+            check_exact=check_exact,
+            rel_tol=rel_tol,
+            abs_tol=abs_tol,
+            categorical_as_str=categorical_as_str,


The nested cases need coverage for an inner Categorical + categorical_as_str

was surprised to see this didn't have an effect

diff --git a/narwhals/testing/asserts/series.py b/narwhals/testing/asserts/series.py index 1521a335d..ca4e29c70 100644 --- a/narwhals/testing/asserts/series.py +++ b/narwhals/testing/asserts/series.py @@ -105,7 +105,6 @@ def assert_series_equal( check_exact=check_exact, rel_tol=rel_tol, abs_tol=abs_tol, - categorical_as_str=categorical_as_str, ) else: _check_approximate_values(left_vals, right_vals, rel_tol=rel_tol, abs_tol=abs_tol) @@ -160,7 +159,6 @@ def _check_exact_values( check_exact: bool, rel_tol: float, abs_tol: float, - categorical_as_str: bool, ) -> None: """Check exact value equality for various data types.""" left_impl = left.implementation @@ -182,7 +180,6 @@ def _check_exact_values( check_exact=check_exact, rel_tol=rel_tol, abs_tol=abs_tol, - categorical_as_str=categorical_as_str, ) _check_list_like(left, right, left_dtype, right_dtype, check_fn=check_fn) # If `_check_list_like` didn't raise, then every nested element is equal @@ -196,7 +193,6 @@ def _check_exact_values( check_exact=check_exact, rel_tol=rel_tol, abs_tol=abs_tol, - categorical_as_str=categorical_as_str, ) _check_struct(left, right, left_dtype, right_dtype, check_fn=check_fn) # If `_check_struct` didn't raise, then every nested element is equal

dangotbanned · 2025-09-06T20:21:03Z

narwhals/testing/asserts/series.py

+def _check_metadata(
+    left: SeriesT, right: SeriesT, *, check_dtypes: bool, check_names: bool
+) -> None:
+    """Check metadata information: implementation, length, dtype, and names."""


How would you feel about moving every function from here onwards into utils?
https://github.com/narwhals-dev/narwhals/blob/9a4b605da3e7ef83555c94c413c8648b39865a92/narwhals/testing/asserts/utils.py

Not a strong preference but slightly in favor of keeping it like this until we are aware that something can be re-used also for dataframes. The reason for this is that say we move _check_metadata, it's very likely that dataframe metadata is composed of different check statements. Then we would end up with _check_series_metadata and _check_frame_metadata.

These functions are internal only, we can always move them around as we please

dangotbanned · 2025-09-06T20:21:34Z

I do hope to get back to reviewing this soon - please nag me if I dont! 🙏

narwhals/testing/asserts/series.py

It falls back to `Series[Any]` already

Will add a thread on `SeriesDetail`

dangotbanned · 2025-09-10T18:13:12Z

narwhals/testing/asserts/utils.py

+SeriesDetail: TypeAlias = Literal[
+    "implementation mismatch",
+    "length mismatch",
+    "dtype mismatch",
+    "name mismatch",
+    "null value mismatch",
+    "exact value mismatch",
+    "values not within tolerance",
+    "nested value mismatch",
+]


refactor(suggestion): Ensure consistent reasons?

I noticed a couple were used more than once - so I thought this might help us keep them consistent in the future? 🙂

If as you've planned, we add assert_frame_equal, then we can probably share some of these between the two

For now it's just a lil bit of autocomplete 😄

I would partially argue that this seems a bit of an overkill?

I can see the advantage of

For now it's just a lil bit of autocomplete

but otherwise if I was developing something and would not know any better, it would feel like I am constrained to use one of those detail value without a clear reason 🧐

You could add a # NOTE by the alias saying to extend it when adding new features?

narwhals/testing/asserts/frame.py

FBruzzesi · 2025-10-02T08:20:27Z

@MarcoGorelli gentle ping to get your review here :)

MarcoGorelli

thanks both, i haven't checked through everything but i'm on board with the idea, if you're both happy with it then feel free to ship it

FBruzzesi added 16 commits August 5, 2025 09:47

WIP: assert series equal

ffb9421

WIP

5839b1c

merge main

df92a53

wait for is_close method

6a0ae06

wrong invert in check_exact

bcab59c

folder structure as polars

69fc160

merge main

7a05d17

WIP unit tests

a07822f

handle nested dtypes with recursion

a1c7e9e

factor out nested checks

e7637ce

merge main

0c137ec

coverage

fe30e5b

line length for docstring

774419a

refactor into subfunctions

20341e4

refactor tests

f3f48b8

add docpage

b61a5a4

FBruzzesi added the enhancement New feature or request label Aug 13, 2025

dangotbanned mentioned this pull request Aug 13, 2025

Tracking: Overhauling the test suite #2959

Open

merge main

d21879d

FBruzzesi mentioned this pull request Aug 13, 2025

fix: Workaround for clip with modin[pyarrow] and cudf backends #2986

Merged

10 tasks

FBruzzesi and others added 8 commits August 15, 2025 14:09

merge main

be0ae09

skip tests with nested dtype for old pandas

f161929

skip pyarrow old versions

bae0a9a

skip pyarrow old version for arrays

546110c

Merge branch 'main' into testing

b6c24d0

merge main

afb47a8

merge main

fde4fc4

use zip_strict

3726fe7

FBruzzesi mentioned this pull request Aug 20, 2025

feat(typing): Make Implementation less opaque #3016

Merged

10 tasks

dangotbanned self-requested a review August 28, 2025 20:23

Merge branch 'main' into testing

44620db

dangotbanned reviewed Aug 29, 2025

View reviewed changes

dangotbanned added 2 commits August 29, 2025 22:06

test(suggestion): Only allow nw.Series

96a9ab1

#2983 (comment)

refactor: factor-in _maybe_apply_preprocessing

288a961

dangotbanned reviewed Aug 29, 2025

View reviewed changes

FBruzzesi added 3 commits August 30, 2025 19:08

add coverage for type error

5d7040b

merge main

72b7cf0

categorical case

4c9f8d9

FBruzzesi changed the title ~~RFC, feat: Add assert_series_equal~~ feat: Add assert_series_equal Sep 4, 2025

FBruzzesi and others added 4 commits September 4, 2025 12:17

correct xfail cases

af5f6d4

Merge branch 'main' into testing

ce7e41e

merge main

14b56cd

skip old pyarrow

9a4b605

dangotbanned reviewed Sep 6, 2025

View reviewed changes

merge main

6433213

FBruzzesi commented Sep 8, 2025

View reviewed changes

narwhals/testing/asserts/series.py Outdated Show resolved Hide resolved

FBruzzesi and others added 4 commits September 8, 2025 21:31

update docstring to use narwhals.Series

43646b4

fix(typing): Avoid type ignore on CheckFn

b81b781

It falls back to `Series[Any]` already

Merge remote-tracking branch 'upstream/main' into testing

1fa0643

refactor(suggestion): Ensure consistent reasons?

a0592e1

Will add a thread on `SeriesDetail`

dangotbanned reviewed Sep 10, 2025

View reviewed changes

dangotbanned reviewed Sep 13, 2025

View reviewed changes

narwhals/testing/asserts/frame.py Outdated Show resolved Hide resolved

FBruzzesi added 2 commits September 14, 2025 11:55

merge main

18b0652

rm assert_frame_equal

153d51f

Merge branch 'main' into testing

fc4390b

MarcoGorelli reviewed Oct 6, 2025

View reviewed changes

	if any(not is_narwhals_series(obj) for obj in (left, right)):
	msg = (
	"Expected `narwhals.Series` instance, found:\n"
	f"[left]: {qualified_type_name(type(left))}\n"
	f"[right]: {qualified_type_name(type(left))}\n\n"
	"Hint: Use `nw.from_native(obj, series_only=True) to convert to `narwhals.Series`"
	)
	raise TypeError(msg)

feat: Add assert_series_equal #2983

Are you sure you want to change the base?

feat: Add assert_series_equal #2983

Uh oh!

Conversation

FBruzzesi commented Aug 13, 2025 • edited by dangotbanned Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What type of PR is this? (check all applicable)

Checklist

If you have comments or can explain your changes, please do so below

Uh oh!

FBruzzesi commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dangotbanned commented Aug 28, 2025

rust impl

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FBruzzesi Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FBruzzesi Aug 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dangotbanned commented Sep 6, 2025

Uh oh!

Uh oh!

dangotbanned Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FBruzzesi Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FBruzzesi commented Oct 2, 2025

Uh oh!

MarcoGorelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

feat: Add `assert_series_equal` #2983

feat: Add `assert_series_equal` #2983

FBruzzesi commented Aug 13, 2025 •

edited by dangotbanned

Loading

FBruzzesi commented Aug 13, 2025 •

edited

Loading

`rust` impl

FBruzzesi Aug 30, 2025 •

edited

Loading

FBruzzesi Aug 31, 2025 •

edited

Loading

dangotbanned Sep 10, 2025 •

edited

Loading

FBruzzesi Sep 10, 2025 •

edited

Loading