Skip to content

Fix broken DeepDiff for dateutils.rrules (#355) #356

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

Merged
merged 5 commits into from
Dec 1, 2022

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants