From dda659b36bc8a3a9f477d6bbc57cb7eb10c614a6 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Wed, 11 Sep 2019 17:07:31 -0400 Subject: [PATCH] bt2: add `__hash__()` method on hashable fields MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Implement the `__hash__()` method of selected fields so that they can be used as keys in dictionaries. This commit only implements the `__hash__()` method of _NumericFieldConst and _StringFieldConst fields after considering the following facts: 1. "[...] the implementation of hashable collections requires that a key’s hash value is immutable (if the object’s hash value changes, it will be in the wrong hash bucket)." [1], 2. Python built-in types bool, int, float, and string are hashable, and sequences and dictionaries are not. So I suggest that only const fields should be hashable as they are immutable, and that Container fields (variant, static array, etc.) should not be hashable (even if they are const) to be consistent with Python built-in types. [1]: https://docs.python.org/3/reference/datamodel.html?highlight=__hash__#object.__hash__ Signed-off-by: Francis Deslauriers Change-Id: I3dc246802d628603a3f06ecd3a87b7e38b3f1e27 Reviewed-on: https://review.lttng.org/c/babeltrace/+/2032 Reviewed-by: Simon Marchi Tested-by: jenkins --- src/bindings/python/bt2/bt2/field.py | 16 ++++++++- tests/bindings/python/bt2/test_field.py | 45 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/bindings/python/bt2/bt2/field.py b/src/bindings/python/bt2/bt2/field.py index 4f89d1e4..1e2ceac3 100644 --- a/src/bindings/python/bt2/bt2/field.py +++ b/src/bindings/python/bt2/bt2/field.py @@ -180,6 +180,9 @@ class _NumericFieldConst(_FieldConst): except Exception: return False + def __hash__(self): + return hash(self._value) + def __rmod__(self, other): return self._extract_value(other) % self._value @@ -242,7 +245,10 @@ class _NumericFieldConst(_FieldConst): class _NumericField(_NumericFieldConst, _Field): - pass + def __hash__(self): + # Non const field are not hashable as their value may be modified + # without changing the underlying Python object. + raise TypeError('unhashable type: \'{}\''.format(self._NAME)) class _IntegralFieldConst(_NumericFieldConst, numbers.Integral): @@ -483,6 +489,9 @@ class _StringFieldConst(_FieldConst): def __bool__(self): return bool(self._value) + def __hash__(self): + return hash(self._value) + def _repr(self): return repr(self._value) @@ -513,6 +522,11 @@ class _StringField(_StringFieldConst, _Field): ) return self + def __hash__(self): + # Non const field are not hashable as their value may be modified + # without changing the underlying Python object. + raise TypeError('unhashable type: \'{}\''.format(self._NAME)) + class _ContainerFieldConst(_FieldConst): def __bool__(self): diff --git a/tests/bindings/python/bt2/test_field.py b/tests/bindings/python/bt2/test_field.py index cdacd7ff..a0ff8e76 100644 --- a/tests/bindings/python/bt2/test_field.py +++ b/tests/bindings/python/bt2/test_field.py @@ -660,6 +660,18 @@ class _TestNumericField: def test_str_op(self): self.assertEqual(str(self._def), str(self._def_value)) + def test_hash_op(self): + with self.assertRaises(TypeError): + hash(self._def) + + def test_const_hash_op(self): + self.assertEqual(hash(self._def_const), hash(self._def_value)) + + def test_const_hash_dict(self): + my_dict = {} + my_dict[self._def_const] = 'my_value' + self.assertEqual(my_dict[self._def_value], 'my_value') + def test_eq_none(self): # Ignore this lint error: # E711 comparison to None should be 'if cond is None:' @@ -1287,6 +1299,10 @@ _inject_numeric_testing_methods(_TestIntegerFieldCommon) class SignedIntegerFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase): + @staticmethod + def _const_value_setter(field): + field.value = 17 + def _create_fc(self, tc): return tc.create_signed_integer_field_class(25) @@ -1297,10 +1313,17 @@ class SignedIntegerFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase): self._def = _create_field(self._tc, self._create_fc(self._tc)) self._def.value = 17 self._def_value = 17 + self._def_const = _create_const_field( + self._tc, self._create_fc(self._tc), self._const_value_setter + ) self._def_new_value = -101 class SignedEnumerationFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase): + @staticmethod + def _const_value_setter(field): + field.value = 17 + def _create_fc(self, tc): fc = tc.create_signed_enumeration_field_class(32) fc.add_mapping('something', bt2.SignedIntegerRangeSet([(17, 17)])) @@ -1318,6 +1341,9 @@ class SignedEnumerationFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase) self._def = _create_field(self._tc, self._create_fc(self._tc)) self._def.value = 17 self._def_value = 17 + self._def_const = _create_const_field( + self._tc, self._create_fc(self._tc), self._const_value_setter + ) self._def_new_value = -101 def test_str_op(self): @@ -1342,6 +1368,10 @@ class SignedEnumerationFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase) class RealFieldTestCase(_TestNumericField, unittest.TestCase): + @staticmethod + def _const_value_setter(field): + field.value = 52.7 + def _create_fc(self, tc): return tc.create_real_field_class() @@ -1349,6 +1379,9 @@ class RealFieldTestCase(_TestNumericField, unittest.TestCase): self._tc = get_default_trace_class() self._field = _create_field(self._tc, self._create_fc(self._tc)) self._def = _create_field(self._tc, self._create_fc(self._tc)) + self._def_const = _create_const_field( + self._tc, self._tc.create_real_field_class(), self._const_value_setter + ) self._def.value = 52.7 self._def_value = 52.7 self._def_new_value = -17.164857 @@ -1541,6 +1574,18 @@ class StringFieldTestCase(unittest.TestCase): self._def_value += to_append self.assertEqual(self._def, self._def_value) + def test_hash_op(self): + with self.assertRaises(TypeError): + hash(self._def) + + def test_const_hash_op(self): + self.assertEqual(hash(self._def_const), hash(self._def_value)) + + def test_const_hash_dict(self): + my_dict = {} + my_dict[self._def_const] = 'my_value' + self.assertEqual(my_dict[self._def_value], 'my_value') + class _TestArrayFieldCommon: def _modify_def(self): -- 2.34.1