Skip to content

TST: reintroduce check_series_type in assert_series_equal #32670

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Mar 16, 2020
Merged

TST: reintroduce check_series_type in assert_series_equal #32670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d847b82
reintroduce check_series_type
martinfleis Mar 12, 2020
e44f44b
add msg and test
jorisvandenbossche Mar 13, 2020
89250c1
fix inverse order
jorisvandenbossche Mar 13, 2020
b8b22d6
Merge remote-tracking branch 'upstream/master' into check_series_type
jorisvandenbossche Mar 16, 2020
2e2586e
fix test
jorisvandenbossche Mar 16, 2020
b0f9cfb
Clean-up comments, use assert_clas_equal
jorisvandenbossche Mar 16, 2020
d07388e
add whatsnew
jorisvandenbossche Mar 16, 2020
8162931
Merge branch 'master' into check_series_type
jorisvandenbossche Mar 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.1.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ Other
- Set operations on an object-dtype :class:`Index` now always return object-dtype results (:issue:`31401`)
- Bug in :meth:`AbstractHolidayCalendar.holidays` when no rules were defined (:issue:`31415`)
- Bug in :meth:`DataFrame.to_records` incorrectly losing timezone information in timezone-aware ``datetime64`` columns (:issue:`32535`)
- Fixed :func:`pandas.testing.assert_series_equal` to correctly raise if left object is a different subclass with ``check_series_type=True`` (:issue:`32670`).
- :meth:`IntegerArray.astype` now supports ``datetime64`` dtype (:issue:32538`)

.. ---------------------------------------------------------------------------
Expand Down
9 changes: 5 additions & 4 deletions pandas/_testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,7 @@ def assert_series_equal(
right,
check_dtype=True,
check_index_type="equiv",
check_series_type=True,
check_less_precise=False,
check_names=True,
check_exact=False,
Expand All @@ -1070,6 +1071,8 @@ def assert_series_equal(
check_index_type : bool or {'equiv'}, default 'equiv'
Whether to check the Index class, dtype and inferred_type
are identical.
check_series_type : bool, default True
Whether to check the Series class is identical.
check_less_precise : bool or int, default False
Specify comparison precision. Only used when check_exact is False.
5 digits (False) or 3 digits (True) after decimal points are compared.
Expand Down Expand Up @@ -1101,10 +1104,8 @@ def assert_series_equal(
# instance validation
_check_isinstance(left, right, Series)

# TODO: There are some tests using rhs is sparse
# lhs is dense. Should use assert_class_equal in future
assert isinstance(left, type(right))
# assert_class_equal(left, right, obj=obj)
if check_series_type:
assert_class_equal(left, right, obj=obj)

# length comparison
if len(left) != len(right):
Expand Down
6 changes: 4 additions & 2 deletions pandas/tests/frame/test_subclass.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,14 @@ def test_subclass_align_combinations(self):

# frame + series
res1, res2 = df.align(s, axis=0)
exp1 = pd.DataFrame(
exp1 = tm.SubclassedDataFrame(
{"a": [1, np.nan, 3, np.nan, 5], "b": [1, np.nan, 3, np.nan, 5]},
index=list("ABCDE"),
)
# name is lost when
exp2 = pd.Series([1, 2, np.nan, 4, np.nan], index=list("ABCDE"), name="x")
exp2 = tm.SubclassedSeries(
[1, 2, np.nan, 4, np.nan], index=list("ABCDE"), name="x"
)

assert isinstance(res1, tm.SubclassedDataFrame)
tm.assert_frame_equal(res1, exp1)
Expand Down
21 changes: 21 additions & 0 deletions pandas/tests/util/test_assert_series_equal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,24 @@ def test_series_equal_categorical_mismatch(check_categorical):
tm.assert_series_equal(s1, s2, check_categorical=check_categorical)
else:
_assert_series_equal_both(s1, s2, check_categorical=check_categorical)


def test_series_equal_series_type():
class MySeries(Series):
pass

s1 = Series([1, 2])
s2 = Series([1, 2])
s3 = MySeries([1, 2])

tm.assert_series_equal(s1, s2, check_series_type=False)
tm.assert_series_equal(s1, s2, check_series_type=True)

tm.assert_series_equal(s1, s3, check_series_type=False)
tm.assert_series_equal(s3, s1, check_series_type=False)

with pytest.raises(AssertionError, match="Series classes are different"):
tm.assert_series_equal(s1, s3, check_series_type=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this test include tm.assert_series_equal(s3, s1, check_series_type=True) (with an xfail))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should but the original check_series_type should also be extended as
isinstance(s3, type(s1)) is True while isinstance(s1, type(s3)) is False. assert_series_equal should probably return the same result no matter the order of left and right Series.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think the fix is outside of the scope of this PR since this is simply reverting changes. but if we are adding a new test, the test should probably test all permutations for completeness. Maybe add a TODO comment in the test instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just fix it here at once?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could maybe remove the comments in assert_series_equal now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just fix it here at once?

@jorisvandenbossche that broke (some) tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, should be easy to fix the test to make proper use of assert_frame_equal, I think (will look at it tomorrow)


with pytest.raises(AssertionError, match="Series classes are different"):
tm.assert_series_equal(s3, s1, check_series_type=True)