Update diff.py

[kor4ik]

Change proposed based on Issue #369
#369

[seperman]

Hi @kor4ik
Thanks for the PR.
My concern is that this breaks the behavior we have with exclude_paths where you have to pass the full path.
When it comes to partial path matching we have the exclude_regex_paths. Perhaps we should follow the same pattern here and add an include_regex_paths and let the user decide how the matching should work.

[kor4ik]

Hi,
Thanks for your comments.
Probably we should take more expanded look here.
The change in line: 455 is not changing any existing logic about exclude_paths or exclude_regex_paths, just suggesting solution for include_paths where User would like to provide the path deeper than the root key.
The case described in issue #369
Moreover, I just tested exclude_paths with the full path and don't see any impact.

[seperman]

Hi,I meant that include_paths currently follows the same pattern as exclude_paths which is taking “prefix” of the path. If we make include_paths not do “prefix” of the path, we are breaking that pattern.Sep DehpourOn Apr 19, 2023, at 12:34 AM, kor4ik ***@***.***> wrote: Hi, Thanks for your comments. Probably we should take more expanded look here. The change in line: 455 is not changing any existing logic about exclude_paths or exclude_regex_paths, just suggesting solution for include_paths where User would like to provide the path deeper than the root key. The case described in issue #369 Moreover, I just tested exclude_paths with the full path and don't see any impact. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>

[kor4ik]

U absolutely right, we should not break common pattern.
include_paths "prefix" behavior is differ and can be level_path or prefix
the direction depends on deep/nest of the dictionary
Could u think of include_paths=[EXMPLE] that would not work with proposed solution ?

[seperman]

@kor4ik We automatically add root to the items in the include_paths.

For example:

include_paths=['foo'] becomes ['root.foo', "root['foo']"] by the time it reaches the line you changed.

As a result it is really a prefix of the path.

My suggestion is to add an include_regex_paths parameter to DeepDiff. It follows the same pattern as the existing exclude_paths_regex. It will solve a superset of cases you are trying to address.

[tim-ilgiz]

I support seperman. It would be very convenient. Just now I wanted to implement such a case myself, so it’s not there out of the box.

[kor4ik]

@seperman
I just tested provided example: include_paths=['foo'] and its working perfect
prefix: "root['foo']" will make condition: prefix in level_path for all level_path values from root to the last nest

Going back to Issue: #369 where we want to care about include_paths does not work with nested dicts

master code line: 455
condition statement: level_path.startswith(prefix)
satisfying only include_paths=['foo'] or "root['foo']"
however include_paths="root['foo']['bar']" is not satisfied
skip = True for level_path = "root['foo']"

preserving all previous functionality and not breaking any patterns
new suggested condition statement: prefix in level_path or level_path in prefix
is satisfying all the cases: include_paths=['foo'] or "root['foo']" or "root['foo']['bar']"
skip = False for level_path = "root['foo']"

Regarding include_regex_paths parameter implementation, in my point of view not directly related Issue: #369 and should be handled as separate Issue
@tim-ilgiz I would suggesting raising new Issue and handling it with different PR

[seperman]

@kor4ik Great point about include_paths="root['foo']['bar']" is not satisfied in the current code.
Can you add a test case for include_paths="root['foo']['bar']" ? Such a test case should fail with our existing code, so it is worth adding it to prevent someone from changing the logic here to something that will make the test case fail.

[kor4ik]

@seperman
Create test_diff_include_paths
60302aa
Issue: #369 fix include_paths does not work with nested dicts

[seperman]

Awesome. Thanks for the tests @kor4ik
Merging. I will keep you posted as soon as it is released.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.97%. Comparing base (e633e37) to head (60302aa).
Report is 233 commits behind head on dev.

Additional details and impacted files

@@ Coverage Diff @@ ## dev #389 +/- ## ======================================= Coverage 98.97% 98.97% ======================================= Files 14 14 Lines 3205 3205 ======================================= Hits 3172 3172 Misses 33 33 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tim-ilgiz]

@seperman I just tested provided example: include_paths=['foo'] and its working perfect prefix: "root['foo']" will make condition: prefix in level_path for all level_path values from root to the last nest

Going back to Issue: #369 where we want to care about include_paths does not work with nested dicts

master code line: 455 condition statement: level_path.startswith(prefix) satisfying only include_paths=['foo'] or "root['foo']" however include_paths="root['foo']['bar']" is not satisfied skip = True for level_path = "root['foo']"

preserving all previous functionality and not breaking any patterns new suggested condition statement: prefix in level_path or level_path in prefix is satisfying all the cases: include_paths=['foo'] or "root['foo']" or "root['foo']['bar']" skip = False for level_path = "root['foo']"

Regarding include_regex_paths parameter implementation, in my point of view not directly related Issue: #369 and should be handled as separate Issue @tim-ilgiz I would suggesting raising new Issue and handling it with different PR

I created an issue #395