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

Add Ignore Iterable Order Option to DeepHash #403

Merged
merged 2 commits into from
Aug 5, 2023

Conversation

bmorck
Copy link

@bmorck bmorck commented Jul 6, 2023

This changes adds a new ignore_iterable_order option to the DeepHash class that allows users to specify whether lists should be sorted or not prior to the hash being computed. By default the option is set to True which preserves existing behavior.

A new test (test_ignore_iterable_order) for this option as been added to tests/test_hash.py.

This also addresses #361

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 @bmorck
Thanks for the PR!
Can you please also update the documentation at docs/deephash_doc.rst?

@@ -190,6 +191,7 @@ def __init__(self,
self.ignore_private_variables = ignore_private_variables
self.encodings = encodings
self.ignore_encoding_errors = ignore_encoding_errors
self.ignore_list_order = ignore_list_order
Copy link
Owner

Choose a reason for hiding this comment

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

Please rename this to ignore_iterable_order to keep it consistent with the rest of parameter names in DeepDiff.

@bmorck bmorck changed the title Add Ignore List Order Option to DeepHash Add Ignore Iterable Order Option to DeepHash Jul 12, 2023
@bmorck bmorck requested a review from seperman July 12, 2023 18:22
@bmorck
Copy link
Author

bmorck commented Jul 12, 2023

@seperman Thanks for the review! Just updated the PR based on your comments

@seperman
Copy link
Owner

seperman commented Aug 5, 2023

Hi @bmorck
Thanks for the prompt fix of the PR. Sorry, it took a few weeks to get back to you.

@codecov-commenter
Copy link

Codecov Report

Merging #403 (b2fcd65) into dev (8951d92) will increase coverage by 0.00%.
Report is 7 commits behind head on dev.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##              dev     #403   +/-   ##
=======================================
  Coverage   98.71%   98.71%           
=======================================
  Files          14       14           
  Lines        3257     3260    +3     
=======================================
+ Hits         3215     3218    +3     
  Misses         42       42           
Files Changed Coverage Δ
deepdiff/deephash.py 99.38% <100.00%> (+<0.01%) ⬆️

@seperman seperman merged commit de56336 into seperman:dev Aug 5, 2023
5 checks passed
@seperman seperman mentioned this pull request Aug 5, 2023
4 tasks
@seperman
Copy link
Owner

seperman commented Sep 1, 2023

@bmorck Thanks for contributing to DeepDiff
DeepDiff 6.4.0 is released and it includes your work! https://zepworks.com/deepdiff/current/

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