Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for bug when diffing two lists with ignore_order and providing compare_func #454

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

sf-tcalhoun
Copy link
Contributor

Made change to diff near line 1128 to change evaluations for lengths from >1 to >0

Scenario:

We have two dictionaries containing lists for
individualNames. Each list contains exactly 2 elements. The effective change is that we are
replacing the 2nd element in the list.
NOTE: This is considered a REPLACEMENT of the second element and not an UPDATE of the element
because we are providing a custom compare_func which will determine matching elements based on
the value of the nameIdentifier field. If the custom compare_func is not used, then
deepdiff.diff will mistakenly treat the difference as being individual field updates for every
field in the second element of the list

See unit test case for old and new diff responses.

@devin13cox
Copy link

devin13cox commented Mar 11, 2024

I think this would also resolve #290, would need to test to confirm.

@sf-tcalhoun
Copy link
Contributor Author

@seperman, any chance you could take a look at this PR. Looks like it might help others as well. Thank you in advance.

@seperman seperman self-assigned this Mar 12, 2024
@seperman seperman changed the base branch from master to dev March 12, 2024 20:00
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.52%. Comparing base (89c5cc2) to head (d705a4b).
Report is 14 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #454   +/-   ##
=======================================
  Coverage   97.52%   97.52%           
=======================================
  Files          14       14           
  Lines        3633     3633           
=======================================
  Hits         3543     3543           
  Misses         90       90           

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

@seperman
Copy link
Owner

Hi @devin13cox
Thanks for the PR! LGTM.
I will keep you posted once I release the new version of DeepDiff that includes your code.

Thanks @sf-tcalhoun for reminding me to take a look at the PR.

@seperman seperman merged commit 71fde30 into seperman:dev Mar 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants