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

Add keys_with_prefix and iterkeys_with_prefix methods #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

yflau
Copy link
Contributor

@yflau yflau commented Aug 16, 2013

I noticed that luikore/hat-trie version adds some methods like hattrie_iter_with_prefix based on dcjones/hat-trie version, so I replace the dcjones/hat-trie with luikore/hat-trie version, and add keys_with_prefix and iterkeys_with_prefix methods based on hattrie_iter_with_prefix. I dont know if want to have such a change? if not, please ignore it.

@kmike
Copy link
Member

kmike commented Aug 16, 2013

Hmm, I don't know. I don't like the callback-based API introduced in luikore's fork - iterator-based API would have been much better. IMHO in long run it will have to be replaced because it is not flexible enough. So it looks like a stopgap solution.

I think that for now I'd leave this PR open without merging it. Let purity beat practicality :) People who need those features could use your fork.

Regarding the wrapper: instead of '(iter)key_with_prefix' methods I'd just provide 'prefix' argument to 'keys' and 'iterkeys', just like it was done in PyTrie, datrie, marisa-trie and DAWG.

@yflau
Copy link
Contributor Author

yflau commented Aug 16, 2013

Actually, I want to add 'prefix' argument reference to PyTrie and others at first, but for clarity and be consistent with hattrie_iter_with_prefix, I add the xxx_with_prefix methods, and I will modify it later :)

@canadaduane
Copy link

Just bumped in to this issue while trying to use the latest hat-trie in triez. It would be great if we could fix this up and merge in.

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