-
Notifications
You must be signed in to change notification settings - Fork 210
feat: add DataFrame.equals and Series.equals #3717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2dfd834
fdec695
5f5af7e
faed3fc
6a6fa3a
deb5259
a89ce90
8b54202
b9c2326
16c7980
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| - columns | ||
| - drop | ||
| - drop_nulls | ||
| - equals | ||
| - estimated_size | ||
| - explode | ||
| - filter | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| - diff | ||
| - drop_nulls | ||
| - dtype | ||
| - equals | ||
| - ewm_mean | ||
| - exp | ||
| - fill_nan | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -553,6 +553,40 @@ def cast(self, dtype: IntoDType) -> Self: | |
| def null_count(self, *, _return_py_scalar: bool = True) -> int: | ||
| return maybe_extract_py_scalar(self.native.null_count, _return_py_scalar) | ||
|
|
||
| def equals( | ||
| self, other: Self, *, check_dtypes: bool, check_names: bool, null_equal: bool | ||
| ) -> bool: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also compare NaNs, which will always be equivalent to one another. You could do something like lhs = ...
rhs = ...
try:
both_nan = pc.fill_null(pc.and_(pc.is_nan(lhs), pc.is_nan(rhs)), False)
except ArrowNotImplementedError:
both_nan = pa.scalar(False) # may need to broadcast this back out to a full array?
values_equal = pc.equal(lhs, rhs)
values_or_nan_equal = pc.fill_null(pc.or_(values_equal, both_nan), False) # no nulls in this array
if null_equal:
# make the extra comparison with null values if null_equal is True
....
return ...
return bool(pc.all(values_or_nan_equal).as_py())
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @camriddell: Thanks for the suggestion ππΌ Note that I slightly rewrote so that the question of broadcasting doesn't even arise in the first place |
||
| if ( | ||
| (check_names and self._name != other._name) | ||
| or (check_dtypes and self.dtype != other.dtype) | ||
| or len(self) != len(other) | ||
| ): | ||
| return False | ||
|
|
||
| lhs = self.native | ||
| rhs = other.native | ||
|
|
||
| if not null_equal and (lhs.null_count or rhs.null_count): | ||
| return False | ||
|
|
||
| try: | ||
| values_equal = pc.equal(lhs, rhs) | ||
| except pa.lib.ArrowNotImplementedError: | ||
| return False | ||
|
|
||
| try: | ||
| both_nan = pc.fill_null(pc.and_(pc.is_nan(lhs), pc.is_nan(rhs)), False) | ||
| values_or_nan_equal = pc.fill_null(pc.or_(values_equal, both_nan), False) | ||
| except pa.lib.ArrowNotImplementedError: | ||
| values_or_nan_equal = pc.fill_null(values_equal, False) | ||
|
|
||
| if null_equal: | ||
| both_null = pc.and_(pc.is_null(lhs), pc.is_null(rhs)) | ||
| result = pc.if_else(both_null, True, values_or_nan_equal) | ||
| return bool(pc.all(result, skip_nulls=False).as_py()) | ||
|
|
||
| return bool(pc.all(values_or_nan_equal).as_py()) | ||
|
|
||
| def head(self, n: int) -> Self: | ||
| if n >= 0: | ||
| return self._with_native(self.native.slice(0, n)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1212,6 +1212,25 @@ def struct(self) -> PandasLikeSeriesStructNamespace: | |
| raise TypeError(msg) | ||
| return PandasLikeSeriesStructNamespace(self) | ||
|
|
||
| def equals( | ||
| self, other: Self, *, check_dtypes: bool, check_names: bool, null_equal: bool | ||
| ) -> bool: | ||
| if check_names and self._name != other._name: | ||
| return False | ||
| if check_dtypes and self.dtype != other.dtype: | ||
| return False | ||
| if not null_equal: | ||
| if len(self) != len(other): | ||
| return False # not needed but can short-circuit the isna() checks below | ||
| if self.native.isna().any() or other.native.isna().any(): | ||
| return False | ||
| other_native = ( | ||
| other.native.astype(self.native.dtype) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure how I feel about this, can't downcasting potentially raise an error? This is where a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed we should avoid manually casting here. >>> import pandas as pd
>>> self = pd.Series([1, 2, 3])
>>> other = pd.Series([*'123'])
>>> self.equals(other)
False
>>> self.equals(other.astype(self.dtype))
True
dtype: int64
>>> other.astype(self.dtype)
0 1
1 2
2 3Instead of directly using >>> pd.Series([1,2,3], dtype="int64") == pd.Series([1.0, 2.0, 3.0], dtype="float64")
0 True
1 True
2 True
dtype: bool
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! But I guess the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for the "not enough" context - that's correct get_supertype is still not here. Hopefully it will eventually get merged #3396
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, so I guess I'll be waiting then until this happens
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please don't! There is not enough consensus on it for now - I brought it up as a nice use case for it, not for blocking you |
||
| if not check_dtypes and self.native.dtype != other.native.dtype | ||
| else other.native | ||
| ) | ||
| return self.native.equals(other_native) | ||
|
|
||
|
|
||
| class _PandasHist(EagerSeriesHist["pd.Series[Any]", "list[float]"]): | ||
| _series: PandasLikeSeries | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| import pytest | ||
|
|
||
| import narwhals as nw | ||
|
|
||
| if TYPE_CHECKING: | ||
| from tests.utils import ConstructorEager | ||
|
|
||
| TBD = None | ||
| _CHECK_NAMES = pytest.mark.xfail( | ||
| strict=True, | ||
| reason=( | ||
| "Bare ChunkedArray carries no name metadata, resulting in empty names - " | ||
| "TBD whether check_names=True should raise for pyarrow in case of empty names" | ||
| ), | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The names should exist on the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the import pyarrow as pa
import narwhals as nw
t = pa.table({"left": [1, 2], "right": [1, 2]})
left = nw.from_native(t["left"], series_only=True)
right = nw.from_native(t["right"], series_only=True)
print(left.name) # ""
print(right.name) # ""
print(left.equals(right, check_names=True)) # True β names are both "", so check passes silently
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see the where the issue arises now. Can you rewrite the test in this fashion so the metadata is preserved appropriately? |
||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("left", "right", "expected"), | ||
| [ | ||
| # --- Same dtype: int --- | ||
| ([1, 2], [1, 2], True), | ||
| # --- Same dtype: float --- | ||
| ([1.1, 2.2], [1.1, 2.2], True), | ||
| # --- Value differs --- | ||
| ([1, 2], [1, 0], False), | ||
| # --- Cross-supertype: float vs int --- | ||
| ([1.0, 2.0], [1, 2], True), | ||
| ([1.1, 2.2], [1, 2], False), | ||
| # --- Cross-supertype: int vs string --- | ||
| ([1, 2], ["1", "2"], False), | ||
| ], | ||
| ) | ||
| def test_series_equals( | ||
| constructor_eager: ConstructorEager, | ||
| left: list, | ||
| right: list, | ||
| expected: bool | None, # noqa: FBT001 | ||
| ) -> None: | ||
| left_native = constructor_eager({"left": left})["left"] | ||
| right_native = constructor_eager({"right": right})["right"] | ||
| left_nw = nw.from_native(left_native, series_only=True) | ||
| right_nw = nw.from_native(right_native, series_only=True) | ||
|
|
||
| result = left_nw.equals(right_nw) | ||
| assert result == expected | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("left", "right", "null_equal", "expected"), | ||
| [ | ||
| # --- Null vs value --- | ||
| ([1, None], [1, 2], True, False), | ||
| ([1, None], [1, 2], False, False), | ||
| # --- Null vs Null --- | ||
| ([1, None], [1, None], True, True), | ||
| ([1, None], [1, None], False, False), | ||
| # --- NaN vs NaN --- | ||
| ([1.0, float("nan")], [1.0, float("nan")], True, True), | ||
| ([1.0, float("nan")], [1.0, float("nan")], False, True), | ||
| # --- NaN vs value --- | ||
| # pandas: False, polars: False, pyarrow: False | ||
| ([1.0, float("nan")], [1.0, 2.0], True, False), | ||
| ], | ||
| ) | ||
| def test_series_equals_null_equal( | ||
| constructor_eager: ConstructorEager, | ||
| left: list, | ||
| right: list, | ||
| null_equal: bool, # noqa: FBT001 | ||
| expected: bool | None, # noqa: FBT001 | ||
| ) -> None: | ||
| left_native = constructor_eager({"left": left})["left"] | ||
| right_native = constructor_eager({"right": right})["right"] | ||
| left_nw = nw.from_native(left_native, series_only=True) | ||
| right_nw = nw.from_native(right_native, series_only=True) | ||
|
|
||
| result = left_nw.equals(right_nw, null_equal=null_equal) | ||
| assert result == expected | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("left", "right", "check_dtypes", "expected"), | ||
| [ | ||
| # --- Same values, different dtype --- | ||
| ([1.1, 2.2], [1, 2], False, False), | ||
| # --- Whole-number floats, check_dtypes=False --- | ||
| ([1.0, 2.0], [1, 2], False, True), | ||
| # --- Whole-number floats, check_dtypes=True --- | ||
| ([1.0, 2.0], [1, 2], True, False), | ||
| ], | ||
| ) | ||
| def test_series_equals_check_dtypes( | ||
| constructor_eager: ConstructorEager, | ||
| left: list, | ||
| right: list, | ||
| check_dtypes: bool, # noqa: FBT001 | ||
| expected: bool | None, # noqa: FBT001 | ||
| ) -> None: | ||
| left_native = constructor_eager({"a": left})["a"] | ||
| right_native = constructor_eager({"a": right})["a"] | ||
| left_nw = nw.from_native(left_native, series_only=True) | ||
| right_nw = nw.from_native(right_native, series_only=True) | ||
|
|
||
| result = left_nw.equals(right_nw, check_dtypes=check_dtypes) | ||
| assert result == expected | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("left_name", "right_name", "check_names", "expected"), | ||
| [ | ||
| # --- Different names --- | ||
| ("left", "right", False, True), | ||
| # pandas: False, polars: False, pyarrow: True (ChunkedArray has no name metadata) | ||
| pytest.param("left", "right", True, TBD, marks=_CHECK_NAMES), | ||
| ], | ||
| ) | ||
| def test_series_equals_check_names( | ||
| constructor_eager: ConstructorEager, | ||
| left_name: str, | ||
| right_name: str, | ||
| check_names: bool, # noqa: FBT001 | ||
| expected: bool | None, # noqa: FBT001 | ||
| ) -> None: | ||
| data = [1, 2] | ||
| left_native = constructor_eager({left_name: data})[left_name] | ||
| right_native = constructor_eager({right_name: data})[right_name] | ||
| left_nw = nw.from_native(left_native, series_only=True) | ||
| right_nw = nw.from_native(right_native, series_only=True) | ||
|
|
||
| result = left_nw.equals(right_nw, check_names=check_names) | ||
| assert result == expected | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we check also the dtype match? I think it's ok to turn off the check as long as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8b54202