Fix for iterable moved items what are found with iterable_compare_func

[dtorres-sf]

Our usage of deepdiff heavily involves iterable_compare_func and we have been unable to upgrade past 6.1.0 because of an issue that was introduced in this changeset.

There are two key changes:

1. When an item is moved, rather than stopping the current iteration, continue to go one level deeper the way item added and removed do.
2. Purposely make moved items relative to t2 rather than t1. This keeps it consistent with other change types. This was originally introduced with this changeset but later got changed around.

This PR should fix this issue:

#414

[dtorres-sf]

I will work on getting unittests fixed soon which may require making some of my changes conditional on either a new flag or iterable_compare_func

[seperman]

Hi @dtorres-sf
Thanks for making the PR.
I may be missing something. What is the benefit of going one level deeper if the item is "moved"? Is it because you want the flexibility when using iterable_compare_func ?
Usually when we report an object as "moved", it means nothing has changed about it except its path, so there is no more need to go deeper.

[dtorres-sf]

@seperman we still need to go deeper, here is an example a basic example:

    {
        'id': 1,
        'value': [1]
    },
    {
        'id': 2,
        'value': [7, 8, 1]
    },
    {
        'id': 3,
        'value': [7, 8],
    },
]

t2 = [
    {
        'id': 2,
        'value': [7, 8]
    },
    {
        'id': 3,
        'value': [7, 8, 1],
    },
    {
        'id': 1,
        'value': [1]
    },
]

In the above example we have both moved top level items as well as changed the moved items. For iterable_compare_func you an object can move but still have changes at the same time.

With these changes here is the output:

{'iterable_item_added': {"root[1]['value'][2]": 1}, 'iterable_item_removed': {"root[0]['value'][2]": 1}, 'iterable_item_moved': {'root[0]': {'new_path': 'root[2]', 'value': {'id': 1, 'value': [1]}}, 'root[1]': {'new_path': 'root[0]', 'value': {'id': 2, 'value': [7, 8]}}, 'root[2]': {'new_path': 'root[1]', 'value': {'id': 3, 'value': [7, 8, 1]}}}}

[dtorres-sf]

@seperman I fixed the unittest by gating one of my changes to only be used for iterable_compare_func. Also updating unittests to be correct.

You can see more examples of why we need to branch deeper in the unittests updates:

https://github.com/seperman/deepdiff/pull/473/files#diff-88c1a4e0a49a73c7956ac53687c9f9d5d710e22516fb76e5773f531ee41cb4acR1956-R1961

These unittests were originally written with the branch deeper in mind - this shows that iterable_compare_func used to go deeper and show the nested values being changed:

deepdiff/tests/test_delta.py

Line 1481 in a3c0684

'values_changed': {"root[2]['val']": {'new_value': 3, 'old_value': 1}},

[seperman]

@dtorres-sf Thanks for fixing the tests. Do you think we should add a parameter that allows the user to decide whether we should go deeper when using iterable_compare_func or not?

[dtorres-sf]

@seperman We could, I am trying to think of the use cases of when they would not want to go deeper. If they don't go deeper they will see that items moved, but not the changes that happened inside the moved items. This effectively misses some of the differences deepdiff is used to find. Do you remember why that continue was added to early exit before going deeper?

The changeset that made the change mentions using difflib but not sure why stopping the branch deeper was added as part of that.

[dtorres-sf]

@seperman Just wanted to follow up here to see if this is a change that you think we could move forward with? If you think it is best to add a parameter for these changes I am fine with that and can get that added.

[seperman]

Hi @dtorres-sf
Sorry for the delay in responding. I'm reviewing your PR now.

[seperman]

@dtorres-sf I think that was added to make it consistent with the rest of the flow where moving means the object has not changed.

[seperman]

I'm going to merge it and add the continue part below when we are not using iterable compare func.

[seperman]

I would make it continue if not self.iterable_compare_func

[seperman]

@dtorres-sf DeepDiff 8.0.0 is published and it includes your contribution. Thank you!

[dtorres-sf]

@seperman Thank you so much!! I will try it out and let you know if there are any issues