From 54630a89eff5568ba6b02278c7b593f08ebf4cc8 Mon Sep 17 00:00:00 2001 From: Martin Winkel Date: Fri, 6 Mar 2020 10:22:02 +0100 Subject: [PATCH 1/4] split and fixturized test_fillna in tests/base/test_ops.py --- pandas/tests/base/test_ops.py | 85 ++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/pandas/tests/base/test_ops.py b/pandas/tests/base/test_ops.py index 8f48d0a3e8378..29da15dfdb5ff 100644 --- a/pandas/tests/base/test_ops.py +++ b/pandas/tests/base/test_ops.py @@ -719,58 +719,63 @@ def test_drop_duplicates_series_vs_dataframe(self): dropped_series = df[column].drop_duplicates(keep=keep) tm.assert_frame_equal(dropped_frame, dropped_series.to_frame()) - def test_fillna(self): + def test_fillna(self, index_or_series_obj): # # GH 11343 # though Index.fillna and Series.fillna has separate impl, # test here to confirm these works as the same - for orig in self.objs: - - o = orig.copy() - values = o.values - - # values will not be changed - result = o.fillna(o.astype(object).values[0]) - if isinstance(o, Index): - tm.assert_index_equal(o, result) - else: - tm.assert_series_equal(o, result) - # check shallow_copied - assert o is not result + obj = index_or_series_obj + if len(obj) == 0: + pytest.skip("Test doesn't make sense on empty data") + elif isinstance(obj, pd.MultiIndex): + pytest.skip("MultiIndex doesn't support isna") - for null_obj in [np.nan, None]: - for orig in self.objs: - o = orig.copy() - klass = type(o) + # values will not be changed + result = obj.fillna(obj.values[0]) + if isinstance(obj, Index): + tm.assert_index_equal(obj, result) + else: + tm.assert_series_equal(obj, result) - if not allow_na_ops(o): - continue + # check shallow_copied + assert obj is not result - if needs_i8_conversion(o): + @pytest.mark.parametrize("null_obj", [np.nan, None]) + def test_fillna_null(self, null_obj, index_or_series_obj): + # # GH 11343 + # though Index.fillna and Series.fillna has separate impl, + # test here to confirm these works as the same + obj = index_or_series_obj + klass = type(obj) - values = o.astype(object).values - fill_value = values[0] - values[0:2] = pd.NaT - else: - values = o.values.copy() - fill_value = o.values[0] - values[0:2] = null_obj + if not allow_na_ops(obj): + pytest.skip(f"{klass} doesn't allow for NA operations") + elif len(obj) < 1: + pytest.skip("Test doesn't make sense on empty data") + elif isinstance(obj, pd.MultiIndex): + pytest.skip(f"MultiIndex can't hold '{null_obj}'") - expected = [fill_value] * 2 + list(values[2:]) + values = obj.values + fill_value = values[0] + expected = values.copy() + if needs_i8_conversion(obj): + values[0:2] = iNaT + expected[0:2] = fill_value + else: + values[0:2] = null_obj + expected[0:2] = fill_value - expected = klass(expected, dtype=orig.dtype) - o = klass(values) + expected = klass(expected) + obj = klass(values) - # check values has the same dtype as the original - assert o.dtype == orig.dtype + result = obj.fillna(fill_value) + if isinstance(obj, Index): + tm.assert_index_equal(result, expected) + else: + tm.assert_series_equal(result, expected) - result = o.fillna(fill_value) - if isinstance(o, Index): - tm.assert_index_equal(result, expected) - else: - tm.assert_series_equal(result, expected) - # check shallow_copied - assert o is not result + # check shallow_copied + assert obj is not result @pytest.mark.skipif(PYPY, reason="not relevant for PyPy") def test_memory_usage(self, index_or_series_obj): From afe6e84ede59059ff665b269d042a912ac201190 Mon Sep 17 00:00:00 2001 From: Martin Winkel Date: Fri, 6 Mar 2020 10:36:07 +0100 Subject: [PATCH 2/4] adjusting isinstance checks to use the ABC versions --- pandas/tests/base/test_ops.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pandas/tests/base/test_ops.py b/pandas/tests/base/test_ops.py index 29da15dfdb5ff..2239b968aee74 100644 --- a/pandas/tests/base/test_ops.py +++ b/pandas/tests/base/test_ops.py @@ -18,6 +18,7 @@ is_object_dtype, needs_i8_conversion, ) +from pandas.core.dtypes.generic import ABCIndex, ABCMultiIndex import pandas as pd from pandas import ( @@ -727,12 +728,12 @@ def test_fillna(self, index_or_series_obj): obj = index_or_series_obj if len(obj) == 0: pytest.skip("Test doesn't make sense on empty data") - elif isinstance(obj, pd.MultiIndex): + elif isinstance(obj, ABCMultiIndex): pytest.skip("MultiIndex doesn't support isna") # values will not be changed result = obj.fillna(obj.values[0]) - if isinstance(obj, Index): + if isinstance(obj, ABCIndex): tm.assert_index_equal(obj, result) else: tm.assert_series_equal(obj, result) @@ -752,7 +753,7 @@ def test_fillna_null(self, null_obj, index_or_series_obj): pytest.skip(f"{klass} doesn't allow for NA operations") elif len(obj) < 1: pytest.skip("Test doesn't make sense on empty data") - elif isinstance(obj, pd.MultiIndex): + elif isinstance(obj, ABCMultiIndex): pytest.skip(f"MultiIndex can't hold '{null_obj}'") values = obj.values @@ -769,7 +770,7 @@ def test_fillna_null(self, null_obj, index_or_series_obj): obj = klass(values) result = obj.fillna(fill_value) - if isinstance(obj, Index): + if isinstance(obj, ABCIndex): tm.assert_index_equal(result, expected) else: tm.assert_series_equal(result, expected) From 343dd6751347b8a7ff67970c762d4b50e8038dfd Mon Sep 17 00:00:00 2001 From: Martin Winkel Date: Fri, 6 Mar 2020 11:01:34 +0100 Subject: [PATCH 3/4] reverted the isinstance checks for ABCIndex --- pandas/tests/base/test_ops.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pandas/tests/base/test_ops.py b/pandas/tests/base/test_ops.py index 2239b968aee74..1b9941fdb64ad 100644 --- a/pandas/tests/base/test_ops.py +++ b/pandas/tests/base/test_ops.py @@ -18,7 +18,7 @@ is_object_dtype, needs_i8_conversion, ) -from pandas.core.dtypes.generic import ABCIndex, ABCMultiIndex +from pandas.core.dtypes.generic import ABCMultiIndex import pandas as pd from pandas import ( @@ -733,7 +733,7 @@ def test_fillna(self, index_or_series_obj): # values will not be changed result = obj.fillna(obj.values[0]) - if isinstance(obj, ABCIndex): + if isinstance(obj, Index): tm.assert_index_equal(obj, result) else: tm.assert_series_equal(obj, result) @@ -770,7 +770,7 @@ def test_fillna_null(self, null_obj, index_or_series_obj): obj = klass(values) result = obj.fillna(fill_value) - if isinstance(obj, ABCIndex): + if isinstance(obj, Index): tm.assert_index_equal(result, expected) else: tm.assert_series_equal(result, expected) From 0a0bdfa1ccc1fe1722a3939eedc64c4f1331ab4a Mon Sep 17 00:00:00 2001 From: Martin Winkel Date: Sun, 8 Mar 2020 18:15:25 +0100 Subject: [PATCH 4/4] test_fillna is executed for empty objs as well now --- pandas/tests/base/test_ops.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pandas/tests/base/test_ops.py b/pandas/tests/base/test_ops.py index 1b9941fdb64ad..9c6e1159fd860 100644 --- a/pandas/tests/base/test_ops.py +++ b/pandas/tests/base/test_ops.py @@ -726,19 +726,21 @@ def test_fillna(self, index_or_series_obj): # test here to confirm these works as the same obj = index_or_series_obj - if len(obj) == 0: - pytest.skip("Test doesn't make sense on empty data") - elif isinstance(obj, ABCMultiIndex): + if isinstance(obj, ABCMultiIndex): pytest.skip("MultiIndex doesn't support isna") # values will not be changed - result = obj.fillna(obj.values[0]) + fill_value = obj.values[0] if len(obj) > 0 else 0 + result = obj.fillna(fill_value) if isinstance(obj, Index): tm.assert_index_equal(obj, result) else: tm.assert_series_equal(obj, result) # check shallow_copied + if isinstance(obj, Series) and len(obj) == 0: + # TODO: GH-32543 + pytest.xfail("Shallow copy for empty Series is bugged") assert obj is not result @pytest.mark.parametrize("null_obj", [np.nan, None])