Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: Different hash for same dictionaries problem #32

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

alexeykomp
Copy link
Contributor

@alexeykomp alexeykomp commented Apr 19, 2024

When merging unhashable types, the hash is calculated based on str(obj).
The problem appears when the hash is being calculated for two dictionaries like
{'foo': 1, 'bar': 2} and
{'bar': 2, 'foo': 1}
In this case, the calculated hashes don't match, though from any other perspective, the dictionaries have no difference.
I'd suggest sorting keys in the dictionary alphabetically on the dict->str conversion.

I'm not a professional programmer so please check whether my logic is valid and if nothing is broken.
make ready-pr passed.

…bj)`. The problem appears when the hash is calculated for two dictionaries like

`a = {'foo': 1, 'bar': 2}` and `b = {'bar': 2, 'foo': 1}.`
In this case, the calculated hashes obviously don't match, though from any other perspective, the dictionaries have no difference.
I suggest sorting keys in the dictionary alphabetically on the dict->str conversion.

I'm not a professional programmer so please check if my logic is valid and nothing is broken
@alexeykomp alexeykomp changed the title Different hash for same dictionaries problem Different hash for same dictionaries problem fix Apr 19, 2024
@alexeykomp alexeykomp changed the title Different hash for same dictionaries problem fix fix: Different hash for same dictionaries problem Apr 23, 2024
Copy link
Owner

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

thanks for the change! in the future adding a unit test would be great (such as in test_list.py).

But I can add that in a follow-up commit. Thank you!

@toumorokoshi toumorokoshi merged commit 7f3fd66 into toumorokoshi:master Aug 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants