From 37c48fff708e17d1a3be7d25d15e73b398ecf73d Mon Sep 17 00:00:00 2001 From: cheqianh Date: Tue, 22 Aug 2023 12:55:00 -0700 Subject: [PATCH] :# This is a combination of 3 commits. Addresses few comments - Refactors some class attributes and renames tests and methods - Removes _copy() - Refactors ionPyDict's constrcutor signature - Changes ion_type to be a class attribute --- amazon/ion/equivalence.py | 6 +-- amazon/ion/ion_py_objects.py | 87 +++++++++++++------------------- tests/test_core_multimap.py | 5 +- tests/test_multimap_IonPyDict.py | 69 +++++++++++++++++++++++++ tests/test_simpleion.py | 10 ++-- 5 files changed, 113 insertions(+), 64 deletions(-) create mode 100644 tests/test_multimap_IonPyDict.py diff --git a/amazon/ion/equivalence.py b/amazon/ion/equivalence.py index bbe7f5db4..b16143b19 100644 --- a/amazon/ion/equivalence.py +++ b/amazon/ion/equivalence.py @@ -26,7 +26,7 @@ from amazon.ion.symbols import SymbolToken -def isinstance_of_ion_nature(obj): +def isinstance_of_ion_nature_or_new_ion_py_objects(obj): return isinstance(obj, _IonNature) or isinstance(obj, IonPyNull_new) \ or isinstance(obj, IonPyDecimal_new) or isinstance(obj, IonPyInt_new) \ or isinstance(obj, IonPyFloat_new) or isinstance(obj, IonPyText_new) \ @@ -70,8 +70,8 @@ def _ion_equals_timestamps_data_model(a, b): def _ion_equals(a, b, timestamp_comparison_func, recursive_comparison_func): """Compares a and b according to the description of the ion_equals method.""" for a, b in ((a, b), (b, a)): # Ensures that operand order does not matter. - if isinstance_of_ion_nature(a): - if isinstance_of_ion_nature(b): + if isinstance_of_ion_nature_or_new_ion_py_objects(a): + if isinstance_of_ion_nature_or_new_ion_py_objects(b): # Both operands have _IonNature. Their IonTypes and annotations must be equivalent. eq = a.ion_type is b.ion_type and _annotations_eq(a, b) else: diff --git a/amazon/ion/ion_py_objects.py b/amazon/ion/ion_py_objects.py index 8501ea2d3..d81362ff0 100644 --- a/amazon/ion/ion_py_objects.py +++ b/amazon/ion/ion_py_objects.py @@ -6,20 +6,18 @@ class IonPyNull_new(object): - ion_annotations = () - ion_type = IonType.NULL # TODO initialized to NULL type first, what's the real type? - __name__ = 'IonPyNull_new' __qualname__ = 'IonPyNull_new' + ion_type = IonType.NULL - def __init__(self, ion_type=None, value=None, annotations=()): - self.ion_type = ion_type + def __init__(self, ion_type=IonType.NULL, value=None, annotations=()): + self.ion_type = ion_type # TODO initialized to NULL type first, what's the real type? self.ion_annotations = annotations def __bool__(self): return False - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -68,18 +66,16 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyDecimal_new(Decimal): - ion_annotations = () - ion_type = IonType.DECIMAL - __name__ = 'IonPyDecimal_new' __qualname__ = 'IonPyDecimal_new' + ion_type = IonType.DECIMAL - def __new__(cls, ion_type, value, annotations=()): + def __new__(cls, ion_type=IonType.DECIMAL, value=None, annotations=()): v = super().__new__(cls, value) v.ion_annotations = annotations return v - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -128,18 +124,16 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyInt_new(int): - ion_annotations = () - ion_type = IonType.INT - __name__ = 'IonPyInt_new' __qualname__ = 'IonPyInt_new' + ion_type = IonType.INT - def __new__(cls, ion_type, value, annotations=()): + def __new__(cls, ion_type=IonType.INT, value=None, annotations=()): v = super().__new__(cls, value) v.ion_annotations = annotations return v - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -188,21 +182,19 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyBool_new(int): - ion_annotations = () - ion_type = IonType.BOOL - __name__ = 'IonPyBool_new' __qualname__ = 'IonPyBool_new' + ion_type = IonType.BOOL def __repr__(self): return str(bool(self)) - def __new__(cls, ion_type, value, annotations=()): + def __new__(cls, ion_type=IonType.BOOL, value=None, annotations=()): v = super().__new__(cls, value) v.ion_annotations = annotations return v - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -251,18 +243,16 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyFloat_new(float): - ion_annotations = () - ion_type = IonType.FLOAT - __name__ = 'IonPyFloat_new' __qualname__ = 'IonPyFloat_new' + ion_type = IonType.FLOAT - def __new__(cls, ion_type, value, annotations=()): + def __new__(cls, ion_type=IonType.FLOAT, value=None, annotations=()): v = super().__new__(cls, value) v.ion_annotations = annotations return v - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -311,18 +301,16 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyText_new(str): - ion_annotations = () - ion_type = IonType.STRING - __name__ = 'IonPyText_new' __qualname__ = 'IonPyText_new' + ion_type = IonType.STRING - def __new__(cls, ion_type=None, value=None, annotations=()): + def __new__(cls, ion_type=IonType.STRING, value=None, annotations=()): v = super().__new__(cls, value) v.ion_annotations = annotations return v - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -371,19 +359,17 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPySymbol_new(SymbolToken): - ion_annotations = () - ion_type = IonType.SYMBOL - __name__ = 'IonPySymbol_new' __qualname__ = 'IonPySymbol_new' + ion_type = IonType.SYMBOL # a good signature: IonPySymbol(ion_type, symbol_token, annotation) - def __new__(cls, ion_type, value, annotations=()): + def __new__(cls, ion_type=IonType.SYMBOL, value=None, annotations=()): v = super().__new__(cls, *value) v.ion_annotations = annotations return v - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -437,19 +423,19 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyList_new(list): - ion_annotations = () - ion_type = IonType.LIST - __name__ = 'IonPyList_new' __qualname__ = 'IonPyList_new' + ion_type = IonType.LIST - def __init__(self, ion_type, value, annotations=()): + def __init__(self, ion_type=IonType.LIST, value=None, annotations=()): + if value is None: + value = [] super().__init__(value) self.ion_annotations = annotations # it's possible to be Sexp self.ion_type = ion_type - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type @@ -498,22 +484,17 @@ def to_event(self, event_type, field_name=None, in_struct=False, depth=None): class IonPyDict_new(MutableMapping): - ion_annotations = () - ion_type = IonType.STRUCT - __name__ = 'IonPyDict_new' __qualname__ = 'IonPyDict_new' + ion_type = IonType.STRUCT - # one good initialization example: my_dict = IonPyDict(ion_type, value, annotations) - def __init__(self, *args, **kwargs): + def __init__(self, ion_type=IonType.STRUCT, value=None, annotations=()): super().__init__() + self.ion_annotations = annotations self.__store = {} - if args is not None: - if len(args) > 1 and args[1] is not None: - for key, value in iter(args[1].items()): - self.__store[key] = [value] - if len(args) > 2 and args[2] is not None: - self.ion_annotations = args[2] + if value is not None: + for key, value in iter(value.items()): + self.__store[key] = [value] def __getitem__(self, key): return self.__store[key][len(self.__store[key]) - 1] # Return only one in order not to break clients @@ -557,7 +538,7 @@ def items(self): output.append((k, v)) return output - def _copy(self): + def __copy__(self): args, kwargs = self._to_constructor_args(self) value = self.__class__(*args, **kwargs) value.ion_type = self.ion_type diff --git a/tests/test_core_multimap.py b/tests/test_core_multimap.py index 226212aa7..7fdc0460f 100644 --- a/tests/test_core_multimap.py +++ b/tests/test_core_multimap.py @@ -1,7 +1,6 @@ from typing import Sequence, NamedTuple -# from amazon.ion.core import Multimap -from amazon.ion.ion_py_objects import IonPyDict_new as Multimap # replace the original multiMap with the new IonPyDict +from amazon.ion.core import Multimap from tests import parametrize @@ -64,7 +63,7 @@ def test_delete_item(item): {"a": 1, "b": 2, "c": [1, 2, {3: 4}]} ) def test_constructor(d): - m = Multimap(None, d) + m = Multimap(d) for k, v in iter(d.items()): assert m[k] == v assert len(m) == len(d) diff --git a/tests/test_multimap_IonPyDict.py b/tests/test_multimap_IonPyDict.py new file mode 100644 index 000000000..17c13b06e --- /dev/null +++ b/tests/test_multimap_IonPyDict.py @@ -0,0 +1,69 @@ +from typing import Sequence, NamedTuple + +from amazon.ion.ion_py_objects import IonPyDict_new as Multimap + +from tests import parametrize + + +class _P(NamedTuple): + pairs: Sequence + expected_all_values: Sequence + expected_single_value: Sequence + expected_total_len: int + + def __str__(self): + return '{name}'.format(name=self.pairs) + + +ALL_DATA = _P( + pairs=[('a', 1), ('a', 2), ('a', 3), ('a', [4, 5, 6]), ('b', 0), ('c', {'x': 'z', 'r': 's'})], + expected_all_values=[('a', [1, 2, 3, [4, 5, 6]]), ('b', [0]), ('c', [{'x': 'z', 'r': 's'}])], + expected_single_value=[('a', [4, 5, 6]), ('b', 0), ('c', {'x': 'z', 'r': 's'})], + expected_total_len=6 +) + + +def _create_multimap_with_items(pairs): + m = Multimap() + for pair in pairs: + m.add_item(pair[0], pair[1]) + return m + + +@parametrize( + ALL_DATA +) +def test_add_item(p): + m = _create_multimap_with_items(p.pairs) + for expected in p.expected_all_values: + assert list([x for x in m.get_all_values(expected[0])]) == expected[1] + for expected in p.expected_single_value: + assert m[expected[0]] == expected[1] + assert p.expected_total_len == len(m) + + +@parametrize( + (ALL_DATA, ["a"], 2), + (ALL_DATA, ["b"], 5), + (ALL_DATA, ["c"], 5), + (ALL_DATA, ["a", "b"], 1), + (ALL_DATA, ["a", "b", "c"], 0) +) +def test_delete_item(item): + m = _create_multimap_with_items(item[0].pairs) + p, items_to_remove, len_after_removal = item + for to_remove in items_to_remove: + del m[to_remove] + assert len(m) == item[2] + + +@parametrize( + {}, + {"a": 1}, + {"a": 1, "b": 2, "c": [1, 2, {3: 4}]} +) +def test_constructor(d): + m = Multimap(None, d) + for k, v in iter(d.items()): + assert m[k] == v + assert len(m) == len(d) diff --git a/tests/test_simpleion.py b/tests/test_simpleion.py index 664faf0a1..76542c772 100644 --- a/tests/test_simpleion.py +++ b/tests/test_simpleion.py @@ -32,7 +32,7 @@ from amazon.ion.core import IonType, IonEvent, IonEventType, OffsetTZInfo, Multimap from amazon.ion.simple_types import IonPyDict, IonPyText, IonPyList, IonPyNull, IonPyBool, IonPyInt, IonPyFloat, \ IonPyDecimal, IonPyTimestamp, IonPyBytes, IonPySymbol, _IonNature -from amazon.ion.equivalence import ion_equals, isinstance_of_ion_nature +from amazon.ion.equivalence import ion_equals, isinstance_of_ion_nature_or_new_ion_py_objects from amazon.ion.simpleion import dump, dumps, load, loads, _ion_type, _FROM_ION_TYPE, _FROM_TYPE_TUPLE_AS_SEXP, \ _FROM_TYPE from amazon.ion.writer_binary_raw import _serialize_symbol, _write_length @@ -272,7 +272,7 @@ def generate_annotated_values_binary(scalars_map, container_map): for value_p in chain(generate_scalars_binary(scalars_map, preceding_symbols=2), generate_containers_binary(container_map, preceding_symbols=2)): obj = value_p.obj - if not isinstance_of_ion_nature(obj): + if not isinstance_of_ion_nature_or_new_ion_py_objects(obj): continue obj.ion_annotations = (_st(u'annot1'), _st(u'annot2'),) annot_length = 2 # 10 and 11 each fit in one VarUInt byte @@ -441,7 +441,7 @@ def generate_annotated_values_text(scalars_map, container_map): for value_p in chain(generate_scalars_text(scalars_map), generate_containers_text(container_map)): obj = value_p.obj - if not isinstance_of_ion_nature(obj): + if not isinstance_of_ion_nature_or_new_ion_py_objects(obj): continue obj.ion_annotations = (_st(u'annot1'), _st(u'annot2'),) @@ -571,7 +571,7 @@ def _to_obj(to_type=None, annotations=(), tuple_as_sexp=False): for obj in roundtrips: obj = _adjust_sids() yield obj, is_binary, indent, False - if not isinstance_of_ion_nature(obj): + if not isinstance_of_ion_nature_or_new_ion_py_objects(obj): ion_type = _ion_type(obj, _FROM_TYPE) yield _to_obj() else: @@ -592,7 +592,7 @@ def _assert_roundtrip(before, after, tuple_as_sexp): # wraps each input value in _IonNature for comparison against the output. def _to_ion_nature(obj): out = obj - if not isinstance_of_ion_nature(out): + if not isinstance_of_ion_nature_or_new_ion_py_objects(out): from_type = _FROM_TYPE_TUPLE_AS_SEXP if tuple_as_sexp else _FROM_TYPE ion_type = _ion_type(out, from_type) out = _FROM_ION_TYPE[ion_type].from_value(ion_type, out)