-
-
Notifications
You must be signed in to change notification settings - Fork 46.6k
hash map #12678
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
base: master
Are you sure you want to change the base?
hash map #12678
Conversation
This is a problem since it causes rapid cycling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for improving it.
Changes look good, I left some cosmetic comments.
As for making _Item
mutable, Immutability gives extra protection in exchange for performance. In this abstract case, I don't have any strong preference, both versions are fine.
# Test resize down when sparse | ||
## Setup: resize up | ||
>>> hm = HashMap(100, capacity_factor=0.75) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we go with size 4? In that case, we could avoid the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this?
# Test resize down when sparse
## Setup: resize up
>>> hm = HashMap(4, capacity_factor=0.75)
>>> len(hm._buckets)
4
>>> hm[0] = 0
>>> hm[1] = 1
>>> hm[2] = 2
>>> len(hm._buckets)
4
>>> hm[3] = 3
>>> len(hm._buckets)
8
## Resize down
>>> del hm[3]
>>> len(hm._buckets)
8
>>> del hm[2]
>>> len(hm._buckets)
4
I've checked that it passes, but I don't know which one I prefer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to keep tests as linear as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait for the 3d opinion.
Co-authored-by: Andrey <[email protected]>
Thank you for the revision!
I understand that the immutable version is faster. That said, performance will be limited in Python either way |
Changes look good. I'll not be active for the next 2 weeks, please don't wait for me. |
Please refer to the commit messages for a detailed breakdown of the changes.
Feedback from the original author, @Cjkjvfnby, would be greatly appreciated!
Describe your change:
Checklist: