Bug in integerNA comparision test

[TomAugspurger]

pandas/pandas/tests/extension/integer/test_integer.py

Lines 327 to 329 in 114f415

	# array
	result = op(s, other)
	expected = pd.Series(op(data._data, other))

says that we're testing the array op, but s is a Series, not an array. Should that be result = pd.Series(op(data, other))?

Series is tested later:

pandas/pandas/tests/extension/integer/test_integer.py

Lines 336 to 341 in 114f415

	# series
	s = pd.Series(data)
	result = op(s, other)
	expected = pd.Series(data._data)
	expected = op(expected, other)

AFAICT.

[TomAugspurger]

A couple side-notes related to these tests

1. It'd be good to add full docstrings for methods that subclasses are expected to override, like _compare_other(self, s, data, op_name, other).
2. It'd a bit hard to resuse code from BaseOpsUtil for the common case of implementing an op. This diff suggests a way to easily use the base class's check, while preserving the default behavior of assuming an array doesn't implement ops.

diff --git a/pandas/tests/extension/base/ops.py b/pandas/tests/extension/base/ops.py
index f7bfdb8ec..6117cc81a 100644
--- a/pandas/tests/extension/base/ops.py
+++ b/pandas/tests/extension/base/ops.py
@@ -20,12 +20,12 @@ class BaseOpsUtil(BaseExtensionTests):
 
         return op
 
-    def check_opname(self, s, op_name, other, exc=NotImplementedError):
+    def check_opname(self, s, op_name, other, exc=Exception):
         op = self.get_op_from_name(op_name)
 
         self._check_op(s, op, other, exc)
 
-    def _check_op(self, s, op, other, exc=NotImplementedError):
+    def _check_op(self, s, op, other, exc=Exception):
         if exc is None:
             result = op(s, other)
             expected = s.combine(other, op)
@@ -34,7 +34,7 @@ class BaseOpsUtil(BaseExtensionTests):
             with pytest.raises(exc):
                 op(s, other)
 
-    def _check_divmod_op(self, s, op, other, exc=NotImplementedError):
+    def _check_divmod_op(self, s, op, other, exc=Exception):
         # divmod has multiple return values, so check separatly
         if exc is None:
             result_div, result_mod = op(s, other)
@@ -51,33 +51,38 @@ class BaseOpsUtil(BaseExtensionTests):
 
 class BaseArithmeticOpsTests(BaseOpsUtil):
     """Various Series and DataFrame arithmetic ops methods."""
+    series_scalar_exc = TypeError
+    frame_scalar_exc = TypeError
+    series_array_exc = TypeError
+    divmod_exc = TypeError
 
     def test_arith_series_with_scalar(self, data, all_arithmetic_operators):
         # series & scalar
         op_name = all_arithmetic_operators
         s = pd.Series(data)
-        self.check_opname(s, op_name, s.iloc[0], exc=TypeError)
+        self.check_opname(s, op_name, s.iloc[0], exc=self.series_scalar_exc)
 
     @pytest.mark.xfail(run=False, reason="_reduce needs implementation")
     def test_arith_frame_with_scalar(self, data, all_arithmetic_operators):
         # frame & scalar
         op_name = all_arithmetic_operators
         df = pd.DataFrame({'A': data})
-        self.check_opname(df, op_name, data[0], exc=TypeError)
+        self.check_opname(df, op_name, data[0], exc=self.frame_scalar_exc)
 
     def test_arith_series_with_array(self, data, all_arithmetic_operators):
         # ndarray & other series
         op_name = all_arithmetic_operators
         s = pd.Series(data)
-        self.check_opname(s, op_name, [s.iloc[0]] * len(s), exc=TypeError)
+        self.check_opname(s, op_name, [s.iloc[0]] * len(s), exc=self.series_array_exc)
 
     def test_divmod(self, data):
         s = pd.Series(data)
-        self._check_divmod_op(s, divmod, 1, exc=TypeError)
-        self._check_divmod_op(1, ops.rdivmod, s, exc=TypeError)
+        self._check_divmod_op(s, divmod, 1, exc=self.divmod_exc)
+        self._check_divmod_op(1, ops.rdivmod, s, exc=self.divmod_exc)
 
     def test_error(self, data, all_arithmetic_operators):
         # invalid ops
+        # What is this testing?
         op_name = all_arithmetic_operators
         with pytest.raises(AttributeError):
             getattr(data, op_name)

[jreback]

it’s just wrapped in a series to use the same testing to compare

[TomAugspurger]

Right, but the op takes place on a Series twice, and never directly on the array. Shouldn't line 328 be

# array
result = op(data, other)

and then later on we do

# series
result = op(s, other)