Skip to content

Conversation

@uwefladrich
Copy link
Contributor

Fix problem with diff'ing dateutils.rrules, but potentially other non-sequence iterables. The underlying issue is that DeepDiff sends objects to difflib.SequenceMatcher without checking that they are actually sequences.

Copy link
Owner

@seperman seperman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @uwefladrich
Thanks for the PR. Left a comment.
Also please add your name and github handle to the AUTHORS.md file.

@uwefladrich uwefladrich marked this pull request as ready for review November 8, 2022 07:33
@uwefladrich uwefladrich requested a review from seperman November 8, 2022 07:35
@seperman seperman changed the base branch from master to dev November 11, 2022 19:22
@seperman seperman changed the base branch from dev to master November 11, 2022 19:24
@seperman seperman changed the base branch from master to dev November 11, 2022 19:24
@seperman
Copy link
Owner

@uwefladrich dateutils needs to be in the requirements-dev-3.7.txt too.

@uwefladrich
Copy link
Contributor Author

Hi @seperman , is there anything I can/should do for this PR at the moment? It seems to be waiting for approval...

@seperman
Copy link
Owner

seperman commented Dec 1, 2022

@uwefladrich Thanks for the reminder.

@codecov-commenter
Copy link

Codecov Report

Merging #356 (2e5f2f1) into dev (093949f) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@ Coverage Diff @@ ## dev #356 +/- ## ========================================== - Coverage 99.42% 99.39% -0.04%  ========================================== Files 14 14 Lines 3143 3143 ========================================== - Hits 3125 3124 -1  - Misses 18 19 +1 
Impacted Files Coverage Δ
deepdiff/diff.py 99.63% <100.00%> (-0.13%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@seperman seperman merged commit 2ccf7db into seperman:dev Dec 1, 2022
@seperman
Copy link
Owner

seperman commented Dec 1, 2022

@uwefladrich Merged. Thanks for contributing to DeepDiff. I will ping you once I cut a release.

@uwefladrich
Copy link
Contributor Author

Great, thanks for considering it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants