Skip to content

Commit

Permalink
Remove memoization of events in _IonNature. (#97)
Browse files Browse the repository at this point in the history
Remove memoization of eents in _IonNature.

This memoization was causing #95 because the IonEvent.field_name field
was being used when writing out struct fields instead of the key of the
field within IonPyDict.  This meant that field with a value that
originated from another IonPyDict to retain field name from the original
IonPyDict.
  • Loading branch information
dlurton committed Sep 4, 2019
1 parent 0add258 commit da74191
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 13 deletions.
16 changes: 5 additions & 11 deletions amazon/ion/simple_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ class _IonNature(object):
User constructed values will generally not set this field.
"""
def __init__(self, *args, **kwargs):
self.ion_event = None
self.ion_type = None
self.ion_annotations = ()

Expand All @@ -61,7 +60,6 @@ def _copy(self):
"""
args, kwargs = self._to_constructor_args(self)
value = self.__class__(*args, **kwargs)
value.ion_event = None
value.ion_type = self.ion_type
value.ion_annotations = self.ion_annotations
return value
Expand All @@ -84,7 +82,6 @@ def from_event(cls, ion_event):
# underlying container will fail.
args, kwargs = (), {}
value = cls(*args, **kwargs)
value.ion_event = ion_event
value.ion_type = ion_event.ion_type
value.ion_annotations = ion_event.annotations
return value
Expand All @@ -103,7 +100,6 @@ def from_value(cls, ion_type, value, annotations=()):
else:
args, kwargs = cls._to_constructor_args(value)
value = cls(*args, **kwargs)
value.ion_event = None
value.ion_type = ion_type
value.ion_annotations = annotations
return value
Expand All @@ -119,13 +115,11 @@ def to_event(self, event_type, field_name=None, depth=None):
Returns:
An IonEvent with the properties from this value.
"""
if self.ion_event is None:
value = self
if isinstance(self, IonPyNull):
value = None
self.ion_event = IonEvent(event_type, ion_type=self.ion_type, value=value, field_name=field_name,
annotations=self.ion_annotations, depth=depth)
return self.ion_event
value = self
if isinstance(self, IonPyNull):
value = None
return IonEvent(event_type, ion_type=self.ion_type, value=value, field_name=field_name,
annotations=self.ion_annotations, depth=depth)


def _ion_type_for(name, base_cls):
Expand Down
2 changes: 0 additions & 2 deletions tests/test_simple_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,9 @@ def test_event_types(p):
output_event = e_scalar(ion_type, event_output)
assert value_event == output_event

assert event_output.ion_event == p.event
assert event_output.ion_type is ion_type
assert p.event.annotations == event_output.ion_annotations

assert value_output.ion_event is to_event_output
assert value_output.ion_type is ion_type
assert p.event.annotations == value_output.ion_annotations

Expand Down
15 changes: 15 additions & 0 deletions tests/test_simpleion.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,18 @@ def test_pretty_print(p):
assert actual_pretty_ion_text == exact_text
for regex_str in regexes:
assert re.search(regex_str, actual_pretty_ion_text, re.M) is not None


# Regression test for issue #95
def test_struct_field():
# pass a dict through simpleion to get a reconstituted dict of Ion values.
struct_a = loads(dumps({u'dont_remember_my_name': 1}))

# copy the value of the "dont_remember_my_name" field to a new struct, which is also passed through simpleion
struct_b = {u'new_name': struct_a[u"dont_remember_my_name"]}
struct_c = loads(dumps(struct_b))

# The bug identified in ion-python#95 is that the name of the original field is somehow preserved.
# verify this no longer happens
assert u'dont_remember_my_name' not in struct_c
assert u'new_name' in struct_c

0 comments on commit da74191

Please sign in to comment.