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

[WIP] OrderedRobinDict #39

Closed
wants to merge 8 commits into from

Conversation

eulerkochy
Copy link
Member

@eulerkochy eulerkochy commented Mar 21, 2020

Work on creating a generic ordered dictionary which can be easily extended to new Dict developed in DataStructures.jl.

I'll be documenting my design process, and later publish that in a blog. In the meantime, bear with the lot of commits, sometimes unnecessary. At the end of the pipeline, all of them will be squashed into meaningful commits, which reflects the essential features of the design process.

Task for now

  • define tombstones and modify iterate accordingly
  • lazy removal of tombstones during rehash!. Implement check_for_rehash and rehash!
  • docstrings
  • tests
  • add to README.md

@eulerkochy eulerkochy requested a review from oxinabox May 23, 2020 17:17
oxinabox
oxinabox previously approved these changes May 25, 2020
@@ -0,0 +1,309 @@
using DataStructures: RobinDict;
Copy link
Member

Choose a reason for hiding this comment

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

But DataStructures has a dependency on OrderedCollections
so you can’t have OrderedCollections depend on DataStructures

The OrderedRobinDict has a RobinDict typed field

@oxinabox oxinabox dismissed their stale review May 25, 2020 12:36

did not look close enough

Comment on lines +80 to +82
ccall(:jl_array_grow_end, Cvoid, (Any, UInt), hk, 1)
nk = length(hk)
@inbounds hk[nk] = key
Copy link
Member

Choose a reason for hiding this comment

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

Why are we doing this instead of just push!(hk, key ?

else
p = sortperm(d.keys; args...)
end
return LittleDict(d.keys[p], d.vals[p])
Copy link
Member

Choose a reason for hiding this comment

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

this should not be a LittleDict it should be a OrderedRobinDict

@fingolfin
Copy link
Member

This is stale and was last touched 4 years ago.

If you still are interested in updating this, please don't hesitate to open a new PR.

@fingolfin fingolfin closed this Nov 24, 2024
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