Skip to content

Commit d8d1766

Browse files
committed
Fix for moving nested tables when using iterable_compare_func.
This fixes issue #540. Before this change the reference params were being swapped right after there was a move. This is because the move needed to have the original paths, but child changes needed the new paths. The problem was that nested moves swapped the reference parameters again after the move was recorded. This made the paths inaccurate since the parent did not have the params swapped but the child did. Instead, we are no longer swapping when building the tree, but rather when we request the paths. The paths will not be swapped for the iterable_item_moved but it will be swapped for all other changes if there was a parent with an iterable_item_moved.
1 parent 69adaf1 commit d8d1766

File tree

3 files changed

+277
-19
lines changed

3 files changed

+277
-19
lines changed

deepdiff/diff.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -952,11 +952,10 @@ def _diff_by_forming_pairs_and_comparing_one_by_one(
952952
self._report_result('iterable_item_moved', change_level, local_tree=local_tree)
953953

954954
if self.iterable_compare_func:
955-
# Intentionally setting j as the first child relationship param in cases of a moved item.
956-
# If the item was moved using an iterable_compare_func then we want to make sure that the index
957-
# is relative to t2.
958-
reference_param1 = j
959-
reference_param2 = i
955+
# Mark additional context denoting that we have moved an item.
956+
# This will allow for correctly setting paths relative to t2 when using an iterable_compare_func
957+
level.additional["moved"] = True
958+
960959
else:
961960
continue
962961

deepdiff/model.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,11 @@ def _from_tree_value_changed(self, tree):
221221

222222
def _from_tree_iterable_item_moved(self, tree):
223223
if 'iterable_item_moved' in tree and self.verbose_level > 1:
224+
224225
for change in tree['iterable_item_moved']:
225-
the_changed = {'new_path': change.path(use_t2=True), 'value': change.t2}
226+
the_changed = {'new_path': change.path(use_t2=True, reporting_move=True), 'value': change.t2}
226227
self['iterable_item_moved'][change.path(
227-
force=FORCE_DEFAULT)] = the_changed
228+
force=FORCE_DEFAULT, use_t2=False, reporting_move=True)] = the_changed
228229

229230
def _from_tree_unprocessed(self, tree):
230231
if 'unprocessed' in tree:
@@ -428,11 +429,11 @@ def _from_tree_iterable_item_moved(self, tree):
428429
if 'iterable_item_moved' in tree:
429430
for change in tree['iterable_item_moved']:
430431
if (
431-
change.up.path(force=FORCE_DEFAULT) not in self["_iterable_opcodes"]
432+
change.up.path(force=FORCE_DEFAULT, reporting_move=True) not in self["_iterable_opcodes"]
432433
):
433-
the_changed = {'new_path': change.path(use_t2=True), 'value': change.t2}
434+
the_changed = {'new_path': change.path(use_t2=True, reporting_move=True), 'value': change.t2}
434435
self['iterable_item_moved'][change.path(
435-
force=FORCE_DEFAULT)] = the_changed
436+
force=FORCE_DEFAULT, reporting_move=True)] = the_changed
436437

437438

438439
class DiffLevel:
@@ -673,7 +674,7 @@ def get_root_key(self, use_t2=False):
673674
return next_rel.param
674675
return notpresent
675676

676-
def path(self, root="root", force=None, get_parent_too=False, use_t2=False, output_format='str'):
677+
def path(self, root="root", force=None, get_parent_too=False, use_t2=False, output_format='str', reporting_move=False):
677678
"""
678679
A python syntax string describing how to descend to this level, assuming the top level object is called root.
679680
Returns None if the path is not representable as a string.
@@ -699,6 +700,9 @@ def path(self, root="root", force=None, get_parent_too=False, use_t2=False, outp
699700
:param output_format: The format of the output. The options are 'str' which is the default and produces a
700701
string representation of the path or 'list' to produce a list of keys and attributes
701702
that produce the path.
703+
704+
:param reporting_move: This should be set to true if and only if we are reporting on iterable_item_moved.
705+
All other cases should leave this set to False.
702706
"""
703707
# TODO: We could optimize this by building on top of self.up's path if it is cached there
704708
cache_key = "{}{}{}{}".format(force, get_parent_too, use_t2, output_format)
@@ -720,7 +724,16 @@ def path(self, root="root", force=None, get_parent_too=False, use_t2=False, outp
720724
# traverse all levels of this relationship
721725
while level and level is not self:
722726
# get this level's relationship object
723-
if use_t2:
727+
if level.additional.get("moved") and not reporting_move:
728+
# To ensure we can properly replay items such as values_changed in items that may have moved, we
729+
# need to make sure that all paths are reported relative to t2 if a level has reported a move.
730+
# If we are reporting a move, the path is already correct and does not need to be swapped.
731+
# Additional context of "moved" is only ever set if using iterable_compare_func and a move has taken place.
732+
level_use_t2 = not use_t2
733+
else:
734+
level_use_t2 = use_t2
735+
736+
if level_use_t2:
724737
next_rel = level.t2_child_rel or level.t1_child_rel
725738
else:
726739
next_rel = level.t1_child_rel or level.t2_child_rel # next relationship object to get a formatted param from

0 commit comments

Comments
 (0)