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

Rework Language.__hash__ #20

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

moi15moi
Copy link

@moi15moi moi15moi commented Sep 7, 2024

Close #19

@georgkrause
Copy link
Owner

@moi15moi Thank you for the patch! This looks valid to me. Can we add a test or two to actually verify the hashes are equal for the same language but different for different ones? I can take over the test, if you are not interested in working on it. But I'd say the examples posted in #19 are already sufficient.

@moi15moi
Copy link
Author

moi15moi commented Sep 9, 2024

Just to be sure that it is clear, before this patch, it we try to add a test to verify the hash is the same when the language is the same AND a test to verify the hash is different when the language isn't the same, the test would already pass due to object caching

The problem "only" happen when we use pickle to save the object. The problem may be reproductable with another way, but I don't know how.

So, how do you want to test it? With saving the object with pickle AND then comparing the hash or by comparing the hash of 2 Language object that have been created in the same process?

@georgkrause
Copy link
Owner

@moi15moi Can't we just set Language._INSTANCES to an empty dict after creating the first object to disable the caching?

@moi15moi
Copy link
Author

moi15moi commented Sep 9, 2024

Yes, that's also a option.

@moi15moi
Copy link
Author

moi15moi commented Sep 9, 2024

Done. Let me know if everything is right for you.

@georgkrause georgkrause merged commit 011067e into georgkrause:main Sep 11, 2024
5 of 6 checks passed
@moi15moi
Copy link
Author

@georgkrause I was wondering if it would be possible to push a new version on PyPi (like 3.4.1) with this fix.

@georgkrause
Copy link
Owner

@moi15moi Good idea, done!

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.

Language.__hash__ is broken
2 participants