-
Notifications
You must be signed in to change notification settings - Fork 243
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
RFC: Added DefaultDict #3
Conversation
* allow DefaultDicts to wrap any Associative type * added tests * added documentation
@lindahua, I've updated this request, and I think it's complete. Let me know what you think. |
# If the Dict implementation changes, this may break. | ||
# Also note that we hash twice here if the key is not in the dictionary: once | ||
# when retrieving, and once when assigning. | ||
function getindex{K,V,F<:Base.Callable}(d::DefaultDict{K,V,F}, key) |
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.
Why not simply use getindex(d::DefaultDict, key) = get(d, key, d.default)
? Isn't this the whole point of get
?
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.
As of right now, get
doesn't modify the dictionary if a key doesn't exist, but getindex
does.
I'm mimicking Python here. I think that choosing whether or not get
modifies the dictionary is arbitrary, and I couldn't think of a good argument against the python implementation. Well, other than the fact that getindex
isn't as efficient... Hmm.
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.
Actually, unless we don't modify the dictionary at all, simply calling get
wouldn't change much, as the logic in this function would then move to get
. The main thing getting in the way of simplifying all of this is that we only want to call d.default()
when the value is not in the dictionary.
Abstractly, what would be useful is a function which, e.g, returns a token
that indicates whether or not a key is in the dictionary, as well as indicating (to the dictionary) the location of the key
if it exists, or where it would go if it doesn't exist. Making such a function public would allow hash values to be returned to the caller.
@kmsquire This looks great to me. I granted the commit privilege to you. Please merge it when you think it is ready. |
Thanks, Dahua. I think it's good to go for now. Cheers! |
This is a
DefaultDict
implementation which mimics the Python implementation of the same.Need to add docs and tests, but the basics are here.Now includes docs and tests.
Some examples (from my updates to the README):
I could make a separate package if that is preferable, but this does seem to at least go with the Accumulator/Counter type (and in fact, could provide an alternate implementation of those). The coding/writing style might be slightly different, so I'm open to any thought/suggestions.