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

Improvements of Trie Structure #756

Merged
merged 7 commits into from
Sep 12, 2021

Conversation

lucidfrontier45
Copy link
Contributor

This PR improved Trie with the following two points.

  1. fix TrieIterator to correctly handle non-ascii characters
  2. add find_prefixes function which is equivalent to prefixes method of https://github.com/pytries/marisa-trie

end
end

partial_path(t::Trie, str::AbstractString) = TrieIterator(t, str)
Base.IteratorSize(::Type{TrieIterator}) = Base.SizeUnknown()

function find_prefixes(t::Trie, str::AbstractString)
Copy link
Member

Choose a reason for hiding this comment

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

this requires a docstring

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added in the latest commit

docs/src/trie.md Outdated

```julia
t = Trie(["A", "ABC", "ABCD", "BCE"])
find_prefixes(t, "ABCDE") # "A", "ABC"
Copy link
Member

Choose a reason for hiding this comment

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

why does this not return ABCD as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's my mistake. I fixed it.

@oxinabox
Copy link
Member

Thanks.

Note that we are currently feature frozen pending #479
and will not be making a 0.19 release with new features until that is done.
(something that I personally do not have time to do, so it is waiting for people who do)

The fix TrieIterator to correctly handle non-ascii characters change is a good bug fix, which we can backpoint to the 0.18.x line.
find_prefixes is not.

@lucidfrontier45
Copy link
Contributor Author

@oxinabox

Thank you for your comment. I updated my commits according to your suggestion.
I also understood the release schedule. I don't have any problem with it.

@oxinabox oxinabox merged commit 8a912ab into JuliaCollections:master Sep 12, 2021
@lucidfrontier45 lucidfrontier45 deleted the trie-improve branch September 13, 2021 01:40
@lucidfrontier45 lucidfrontier45 restored the trie-improve branch September 13, 2021 01:40
@lucidfrontier45 lucidfrontier45 deleted the trie-improve branch July 16, 2022 01:26
ReubenJ added a commit to ReubenJ/DataStructures.jl that referenced this pull request Jul 8, 2024
Squashed version of commits in JuliaCollections#756 with relevant changes only.

Backport to v0.18, meaning we're not including `find_prefixes`.

Co-authored-by: Du Shiqiao <lucidfrontier.45@gmail.com>
Co-authored-by: Reuben Gardos Reid <5456207+ReubenJ@users.noreply.github.com>
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.

2 participants