bt2: value.py: numeric classes: remove __i*__ operators

Those operators do a binary operation to a value object and another
operand, and then set the result as the value object's value.

Now that I think about it, it is weird to do something like:

    a = bt2.UnsignedIntegerValue(23)
    a += 17.5

and that the value of `a` becomes 40. This is not a problem for a native
Python object because `a` does not name the same object after the
operation:

    a = 23
    type(a)     # int
    id(a)       # 140669243488160
    a += 17.5
    type(a)     # float
    id(a)       # 140669043054920

but we can't do this with our value object: this is internal Python's
magic.

So I believe we should be more strict. But this means you can't do:

    a = bt2.UnsignedIntegerValue(23)
    a += 17.5

anymore (a `TypeError` exception would be raised).

This means a lot of specific automatically injected tests must
conditionally exist (depending on whether or not the test is possible
with the two operands, whereas it's always possible now).

Instead of doing this work, I choose to completely remove the support
for "self" operators. We can add useful ones in the future when we have
time, but I don't even think they are super useful: because a value
object becomes conceptually frozen as soon as you use it somehow,
there's no point in doing `+=`, `-=`, `**=`, `<<=`, and so on. For those
rare cases, you can always do:

    a = bt2.UnsignedIntegerValue(23)
    a = a + 17

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Icd5f0a9d0232c2f7ba91133561235fec7526083b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1566
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>

diff --git a/src/bindings/python/bt2/bt2/value.py b/src/bindings/python/bt2/bt2/value.py
index 9cdad357c3a2605cb962711a8a4153381ec834b5..4fbe799286683b2ab3f0c2bd4fb289b0c0802392 100644 (file)
--- a/src/bindings/python/bt2/bt2/value.py
+++ b/src/bindings/python/bt2/bt2/value.py
@@ -197,34 +197,6 @@ class _NumericValue(_Value):
     def __rpow__(self, base):
         return self._extract_value(base) ** self._value
 
-    def __iadd__(self, other):
-        self.value = self + other
-        return self
-
-    def __isub__(self, other):
-        self.value = self - other
-        return self
-
-    def __imul__(self, other):
-        self.value = self * other
-        return self
-
-    def __itruediv__(self, other):
-        self.value = self / other
-        return self
-
-    def __ifloordiv__(self, other):
-        self.value = self // other
-        return self
-
-    def __imod__(self, other):
-        self.value = self % other
-        return self
-
-    def __ipow__(self, other):
-        self.value = self ** other
-        return self
-
 
 class _IntegralValue(_NumericValue, numbers.Integral):
     def __lshift__(self, other):
@@ -260,26 +232,6 @@ class _IntegralValue(_NumericValue, numbers.Integral):
     def __invert__(self):
         return ~self._value
 
-    def __ilshift__(self, other):
-        self.value = self << other
-        return self
-
-    def __irshift__(self, other):
-        self.value = self >> other
-        return self
-
-    def __iand__(self, other):
-        self.value = self & other
-        return self
-
-    def __ixor__(self, other):
-        self.value = self ^ other
-        return self
-
-    def __ior__(self, other):
-        self.value = self | other
-        return self
-
 
 class _RealValue(_NumericValue, numbers.Real):
     pass

diff --git a/tests/bindings/python/bt2/test_value.py b/tests/bindings/python/bt2/test_value.py
index 8f886c7233afb4cc73047f2ee34f15ba244bd35f..2d6e25109d7a3e2c6c71bb9d18664ed4c7084f0c 100644 (file)
--- a/tests/bindings/python/bt2/test_value.py
+++ b/tests/bindings/python/bt2/test_value.py
@@ -164,54 +164,6 @@ class _TestNumericValue(_TestCopySimple):
         with self.assertRaises(TypeError):
             op(self._def, None)
 
-    def _test_ibinop_value(self, op, rhs):
-        r, rv = self._binop(op, rhs)
-
-        if r is None:
-            return
-
-        # The inplace operators are special for value objects because
-        # they do not return a new, immutable object like it's the case
-        # for Python numbers. In Python, `a += 2`, where `a` is a number
-        # object, assigns a new number object reference to `a`, dropping
-        # the old reference. Since BT's value objects are mutable, we
-        # modify their internal value with the inplace operators. This
-        # means however that we can lose data in the process, for
-        # example:
-        #
-        #     int_value_obj += 3.3
-        #
-        # Here, if `int_value_obj` is a Python `int` with the value 2,
-        # it would be a `float` object after this, holding the value
-        # 5.3. In our case, if `int_value_obj` is an integer value
-        # object, 3.3 is converted to an `int` object (3) and added to
-        # the current value of `int_value_obj`, so after this the value
-        # of the object is 5. This does not compare to 5.3, which is
-        # why we also use the `int()` type here.
-        if isinstance(self._def, bt2._IntegerValue):
-            rv = int(rv)
-
-        self.assertEqual(r, rv)
-
-    def _test_ibinop_type(self, op, rhs):
-        r, rv = self._binop(op, rhs)
-
-        if r is None:
-            return
-
-        self.assertIs(r, self._def)
-
-    def _test_ibinop_invalid_unknown(self, op):
-        class A:
-            pass
-
-        with self.assertRaises(TypeError):
-            op(self._def, A())
-
-    def _test_ibinop_invalid_none(self, op):
-        with self.assertRaises(TypeError):
-            op(self._def, None)
-
     def _test_binop_rhs_false(self, test_cb, op):
         test_cb(op, False)
 
@@ -422,90 +374,6 @@ class _TestNumericValue(_TestCopySimple):
     def _test_binop_lhs_value_same_zero_vfloat(self, op):
         self._test_binop_rhs_zero_vfloat(self._test_binop_lhs_value_same, op)
 
-    def _test_ibinop_type_false(self, op):
-        self._test_binop_rhs_false(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_true(self, op):
-        self._test_binop_rhs_true(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_pos_int(self, op):
-        self._test_binop_rhs_pos_int(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_neg_int(self, op):
-        self._test_binop_rhs_neg_int(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_zero_int(self, op):
-        self._test_binop_rhs_zero_int(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_pos_vint(self, op):
-        self._test_binop_rhs_pos_vint(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_neg_vint(self, op):
-        self._test_binop_rhs_neg_vint(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_zero_vint(self, op):
-        self._test_binop_rhs_zero_vint(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_pos_float(self, op):
-        self._test_binop_rhs_pos_float(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_neg_float(self, op):
-        self._test_binop_rhs_neg_float(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_zero_float(self, op):
-        self._test_binop_rhs_zero_float(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_pos_vfloat(self, op):
-        self._test_binop_rhs_pos_vfloat(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_neg_vfloat(self, op):
-        self._test_binop_rhs_neg_vfloat(self._test_ibinop_type, op)
-
-    def _test_ibinop_type_zero_vfloat(self, op):
-        self._test_binop_rhs_zero_vfloat(self._test_ibinop_type, op)
-
-    def _test_ibinop_value_false(self, op):
-        self._test_binop_rhs_false(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_true(self, op):
-        self._test_binop_rhs_true(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_pos_int(self, op):
-        self._test_binop_rhs_pos_int(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_neg_int(self, op):
-        self._test_binop_rhs_neg_int(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_zero_int(self, op):
-        self._test_binop_rhs_zero_int(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_pos_vint(self, op):
-        self._test_binop_rhs_pos_vint(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_neg_vint(self, op):
-        self._test_binop_rhs_neg_vint(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_zero_vint(self, op):
-        self._test_binop_rhs_zero_vint(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_pos_float(self, op):
-        self._test_binop_rhs_pos_float(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_neg_float(self, op):
-        self._test_binop_rhs_neg_float(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_zero_float(self, op):
-        self._test_binop_rhs_zero_float(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_pos_vfloat(self, op):
-        self._test_binop_rhs_pos_vfloat(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_neg_vfloat(self, op):
-        self._test_binop_rhs_neg_vfloat(self._test_ibinop_value, op)
-
-    def _test_ibinop_value_zero_vfloat(self, op):
-        self._test_binop_rhs_zero_vfloat(self._test_ibinop_value, op)
-
     def test_bool_op(self):
         self.assertEqual(bool(self._def), bool(self._def_value))
 
@@ -562,22 +430,6 @@ _BINOPS = (
 )
 
 
-_IBINOPS = (
-    ('iadd', operator.iadd),
-    ('iand', operator.iand),
-    ('ifloordiv', operator.ifloordiv),
-    ('ilshift', operator.ilshift),
-    ('imod', operator.imod),
-    ('imul', operator.imul),
-    ('ior', operator.ior),
-    ('ipow', operator.ipow),
-    ('irshift', operator.irshift),
-    ('isub', operator.isub),
-    ('itruediv', operator.itruediv),
-    ('ixor', operator.ixor),
-)
-
-
 _UNARYOPS = (
     ('neg', operator.neg),
     ('pos', operator.pos),
@@ -598,9 +450,6 @@ def _inject_numeric_testing_methods(cls, has_neg=True):
     def test_binop_name(suffix):
         return 'test_binop_{}_{}'.format(name, suffix)
 
-    def test_ibinop_name(suffix):
-        return 'test_ibinop_{}_{}'.format(name, suffix)
-
     def test_unaryop_name(suffix):
         return 'test_unaryop_{}_{}'.format(name, suffix)
 
@@ -678,45 +527,6 @@ def _inject_numeric_testing_methods(cls, has_neg=True):
         setattr(cls, test_unaryop_name('addr_same'), partialmethod(_TestNumericValue._test_unaryop_addr_same, op=unaryop))
         setattr(cls, test_unaryop_name('value_same'), partialmethod(_TestNumericValue._test_unaryop_value_same, op=unaryop))
 
-    # inject testing methods for each inplace binary operation
-    for name, ibinop in _IBINOPS:
-        setattr(cls, test_ibinop_name('invalid_unknown'), partialmethod(_TestNumericValue._test_ibinop_invalid_unknown, op=ibinop))
-        setattr(cls, test_ibinop_name('invalid_none'), partialmethod(_TestNumericValue._test_ibinop_invalid_none, op=ibinop))
-        setattr(cls, test_ibinop_name('type_true'), partialmethod(_TestNumericValue._test_ibinop_type_true, op=ibinop))
-        setattr(cls, test_ibinop_name('value_true'), partialmethod(_TestNumericValue._test_ibinop_value_true, op=ibinop))
-        setattr(cls, test_ibinop_name('type_pos_int'), partialmethod(_TestNumericValue._test_ibinop_type_pos_int, op=ibinop))
-        setattr(cls, test_ibinop_name('type_pos_vint'), partialmethod(_TestNumericValue._test_ibinop_type_pos_vint, op=ibinop))
-        setattr(cls, test_ibinop_name('value_pos_int'), partialmethod(_TestNumericValue._test_ibinop_value_pos_int, op=ibinop))
-        setattr(cls, test_ibinop_name('value_pos_vint'), partialmethod(_TestNumericValue._test_ibinop_value_pos_vint, op=ibinop))
-
-        if has_neg:
-            setattr(cls, test_ibinop_name('type_neg_int'), partialmethod(_TestNumericValue._test_ibinop_type_neg_int, op=ibinop))
-            setattr(cls, test_ibinop_name('type_neg_vint'), partialmethod(_TestNumericValue._test_ibinop_type_neg_vint, op=ibinop))
-            setattr(cls, test_ibinop_name('value_neg_int'), partialmethod(_TestNumericValue._test_ibinop_value_neg_int, op=ibinop))
-            setattr(cls, test_ibinop_name('value_neg_vint'), partialmethod(_TestNumericValue._test_ibinop_value_neg_vint, op=ibinop))
-
-        setattr(cls, test_ibinop_name('type_false'), partialmethod(_TestNumericValue._test_ibinop_type_false, op=ibinop))
-        setattr(cls, test_ibinop_name('value_false'), partialmethod(_TestNumericValue._test_ibinop_value_false, op=ibinop))
-        setattr(cls, test_ibinop_name('type_zero_int'), partialmethod(_TestNumericValue._test_ibinop_type_zero_int, op=ibinop))
-        setattr(cls, test_ibinop_name('type_zero_vint'), partialmethod(_TestNumericValue._test_ibinop_type_zero_vint, op=ibinop))
-        setattr(cls, test_ibinop_name('value_zero_int'), partialmethod(_TestNumericValue._test_ibinop_value_zero_int, op=ibinop))
-        setattr(cls, test_ibinop_name('value_zero_vint'), partialmethod(_TestNumericValue._test_ibinop_value_zero_vint, op=ibinop))
-        setattr(cls, test_ibinop_name('type_pos_float'), partialmethod(_TestNumericValue._test_ibinop_type_pos_float, op=ibinop))
-
-        if has_neg:
-            setattr(cls, test_ibinop_name('type_neg_float'), partialmethod(_TestNumericValue._test_ibinop_type_neg_float, op=ibinop))
-            setattr(cls, test_ibinop_name('type_neg_vfloat'), partialmethod(_TestNumericValue._test_ibinop_type_neg_vfloat, op=ibinop))
-            setattr(cls, test_ibinop_name('value_neg_float'), partialmethod(_TestNumericValue._test_ibinop_value_neg_float, op=ibinop))
-            setattr(cls, test_ibinop_name('value_neg_vfloat'), partialmethod(_TestNumericValue._test_ibinop_value_neg_vfloat, op=ibinop))
-
-        setattr(cls, test_ibinop_name('type_pos_vfloat'), partialmethod(_TestNumericValue._test_ibinop_type_pos_vfloat, op=ibinop))
-        setattr(cls, test_ibinop_name('value_pos_float'), partialmethod(_TestNumericValue._test_ibinop_value_pos_float, op=ibinop))
-        setattr(cls, test_ibinop_name('value_pos_vfloat'), partialmethod(_TestNumericValue._test_ibinop_value_pos_vfloat, op=ibinop))
-        setattr(cls, test_ibinop_name('type_zero_float'), partialmethod(_TestNumericValue._test_ibinop_type_zero_float, op=ibinop))
-        setattr(cls, test_ibinop_name('type_zero_vfloat'), partialmethod(_TestNumericValue._test_ibinop_type_zero_vfloat, op=ibinop))
-        setattr(cls, test_ibinop_name('value_zero_float'), partialmethod(_TestNumericValue._test_ibinop_value_zero_float, op=ibinop))
-        setattr(cls, test_ibinop_name('value_zero_vfloat'), partialmethod(_TestNumericValue._test_ibinop_value_zero_vfloat, op=ibinop))
-
 
 class CreateValueFuncTestCase(unittest.TestCase):
     def test_create_none(self):