Skip to content

REF/TST: remove overriding tm.assert_foo pattern #54355

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

Merged
merged 5 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 0 additions & 9 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,6 @@ repos:
|default_rng\(\)
files: ^pandas/tests/
types_or: [python, cython, rst]
- id: unwanted-patterns-in-ea-tests
name: Unwanted patterns in EA tests
language: pygrep
entry: |
(?x)
tm.assert_(series|frame)_equal
files: ^pandas/tests/extension/base/
exclude: ^pandas/tests/extension/base/base\.py$
types_or: [python, cython, rst]
- id: unwanted-patterns-in-cython
name: Unwanted patterns in Cython code
language: pygrep
Expand Down
3 changes: 2 additions & 1 deletion pandas/tests/arrays/integer/test_comparison.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

import pandas as pd
import pandas._testing as tm
from pandas.tests.arrays.masked_shared import (
ComparisonOps,
NumericOps,
Expand All @@ -25,7 +26,7 @@ def test_compare_to_int(self, dtype, comparison_op):
expected = method(2).astype("boolean")
expected[s2.isna()] = pd.NA

self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)


def test_equals():
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/arrays/masked_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def test_compare_to_string(self, dtype):
result = ser == "a"
expected = pd.Series([False, pd.NA], dtype="boolean")

self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

def test_ufunc_with_out(self, dtype):
arr = pd.array([1, 2, 3], dtype=dtype)
Expand Down
7 changes: 0 additions & 7 deletions pandas/tests/extension/base/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@ class TestMyDtype(BaseDtypeTests):
``BaseDtypeTests``. pytest's fixture discover will supply your ``dtype``
wherever the test requires it. You're free to implement additional tests.

All the tests in these modules use ``self.assert_frame_equal`` or
``self.assert_series_equal`` for dataframe or series comparisons. By default,
they use the usual ``pandas.testing.assert_frame_equal`` and
``pandas.testing.assert_series_equal``. You can override the checks used
by defining the staticmethods ``assert_frame_equal`` and
``assert_series_equal`` on your base test class.

"""
from pandas.tests.extension.base.accumulate import BaseAccumulateTests # noqa: F401
from pandas.tests.extension.base.casting import BaseCastingTests # noqa: F401
Expand Down
3 changes: 2 additions & 1 deletion pandas/tests/extension/base/accumulate.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

import pandas as pd
import pandas._testing as tm
from pandas.tests.extension.base.base import BaseExtensionTests


Expand All @@ -20,7 +21,7 @@ def check_accumulate(self, s, op_name, skipna):
)

expected = getattr(s.astype("float64"), op_name)(skipna=skipna)
self.assert_series_equal(result, expected, check_dtype=False)
tm.assert_series_equal(result, expected, check_dtype=False)

@pytest.mark.parametrize("skipna", [True, False])
def test_accumulate_series_raises(self, data, all_numeric_accumulations, skipna):
Expand Down
17 changes: 1 addition & 16 deletions pandas/tests/extension/base/base.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,2 @@
import pandas._testing as tm


class BaseExtensionTests:
# classmethod and different signature is needed
# to make inheritance compliant with mypy
@classmethod
def assert_equal(cls, left, right, **kwargs):
return tm.assert_equal(left, right, **kwargs)

@classmethod
def assert_series_equal(cls, left, right, *args, **kwargs):
return tm.assert_series_equal(left, right, *args, **kwargs)

@classmethod
def assert_frame_equal(cls, left, right, *args, **kwargs):
return tm.assert_frame_equal(left, right, *args, **kwargs)
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

This removal is not backward compatible for extensionarrays tests that inherit BaseExtensionTests. IMO this should be deprecated instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a downstream EA that is actually affected or is this just in principal?

If the latter, I'm inclined to be more aggressive with changing the tests than the non-test code. I don't want to get in the business of writing deprecation tests for the tests.

Copy link
Member

Choose a reason for hiding this comment

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

Every downstream EA that reuses those tests can be affected if they use the same pattern in those classes (eg in a test they override). See the two linked PRs below for two such examples.

Now, I think the question is: is there any downstream EA implementation that actually makes use of those test class methods (i.e. by overriding them, and thus not relying purely on the base tm.assert_.._equal functions). That I don't know (the two examples of affected projects don't do this, and so for them it's an easy fix)

Copy link
Member

Choose a reason for hiding this comment

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

Looking at some know downstream EAs, there are a few others that will be affected as well in the sense of having to replace self.assert_.. with tm.assert_.., but I didn't see any public one that actually overrides those methods.

10 changes: 5 additions & 5 deletions pandas/tests/extension/base/casting.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_tolist(self, data):
def test_astype_str(self, data):
result = pd.Series(data[:5]).astype(str)
expected = pd.Series([str(x) for x in data[:5]], dtype=str)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize(
"nullable_string_dtype",
Expand All @@ -62,22 +62,22 @@ def test_astype_string(self, data, nullable_string_dtype):
[str(x) if not isinstance(x, bytes) else x.decode() for x in data[:5]],
dtype=nullable_string_dtype,
)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

def test_to_numpy(self, data):
expected = np.asarray(data)

result = data.to_numpy()
self.assert_equal(result, expected)
tm.assert_equal(result, expected)

result = pd.Series(data).to_numpy()
self.assert_equal(result, expected)
tm.assert_equal(result, expected)

def test_astype_empty_dataframe(self, dtype):
# https://github.com/pandas-dev/pandas/issues/33113
df = pd.DataFrame()
result = df.astype(dtype)
self.assert_frame_equal(result, df)
tm.assert_frame_equal(result, df)

@pytest.mark.parametrize("copy", [True, False])
def test_astype_own_type(self, data, copy):
Expand Down
22 changes: 11 additions & 11 deletions pandas/tests/extension/base/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,27 @@ def test_series_constructor(self, data):
def test_series_constructor_no_data_with_index(self, dtype, na_value):
result = pd.Series(index=[1, 2, 3], dtype=dtype)
expected = pd.Series([na_value] * 3, index=[1, 2, 3], dtype=dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

# GH 33559 - empty index
result = pd.Series(index=[], dtype=dtype)
expected = pd.Series([], index=pd.Index([], dtype="object"), dtype=dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

def test_series_constructor_scalar_na_with_index(self, dtype, na_value):
result = pd.Series(na_value, index=[1, 2, 3], dtype=dtype)
expected = pd.Series([na_value] * 3, index=[1, 2, 3], dtype=dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

def test_series_constructor_scalar_with_index(self, data, dtype):
scalar = data[0]
result = pd.Series(scalar, index=[1, 2, 3], dtype=dtype)
expected = pd.Series([scalar] * 3, index=[1, 2, 3], dtype=dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

result = pd.Series(scalar, index=["foo"], dtype=dtype)
expected = pd.Series([scalar], index=["foo"], dtype=dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize("from_series", [True, False])
def test_dataframe_constructor_from_dict(self, data, from_series):
Expand Down Expand Up @@ -91,19 +91,19 @@ def test_from_dtype(self, data):

expected = pd.Series(data)
result = pd.Series(list(data), dtype=dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

result = pd.Series(list(data), dtype=str(dtype))
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

# gh-30280

expected = pd.DataFrame(data).astype(dtype)
result = pd.DataFrame(list(data), dtype=dtype)
self.assert_frame_equal(result, expected)
tm.assert_frame_equal(result, expected)

result = pd.DataFrame(list(data), dtype=str(dtype))
self.assert_frame_equal(result, expected)
tm.assert_frame_equal(result, expected)

def test_pandas_array(self, data):
# pd.array(extension_array) should be idempotent...
Expand All @@ -114,15 +114,15 @@ def test_pandas_array_dtype(self, data):
# ... but specifying dtype will override idempotency
result = pd.array(data, dtype=np.dtype(object))
expected = pd.arrays.NumpyExtensionArray(np.asarray(data, dtype=object))
self.assert_equal(result, expected)
tm.assert_equal(result, expected)

def test_construct_empty_dataframe(self, dtype):
# GH 33623
result = pd.DataFrame(columns=["a"], dtype=dtype)
expected = pd.DataFrame(
{"a": pd.array([], dtype=dtype)}, index=pd.RangeIndex(0)
)
self.assert_frame_equal(result, expected)
tm.assert_frame_equal(result, expected)

def test_empty(self, dtype):
cls = dtype.construct_array_type()
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/extension/base/dim2.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_frame_from_2d_array(self, data):

df = pd.DataFrame(arr2d)
expected = pd.DataFrame({0: arr2d[:, 0], 1: arr2d[:, 1]})
self.assert_frame_equal(df, expected)
tm.assert_frame_equal(df, expected)

def test_swapaxes(self, data):
arr2d = data.repeat(2).reshape(-1, 2)
Expand Down
5 changes: 3 additions & 2 deletions pandas/tests/extension/base/dtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import pytest

import pandas as pd
import pandas._testing as tm
from pandas.api.types import (
infer_dtype,
is_object_dtype,
Expand Down Expand Up @@ -66,11 +67,11 @@ def test_check_dtype(self, data):

expected = pd.Series([True, True, False, False], index=list("ABCD"))

self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

expected = pd.Series([True, True, False, False], index=list("ABCD"))
result = df.dtypes.apply(str) == str(dtype)
self.assert_series_equal(result, expected)
tm.assert_series_equal(result, expected)

def test_hashable(self, dtype):
hash(dtype) # no error
Expand Down
Loading