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

Tidy up sais.rs #48

Closed
ajalab opened this issue Jan 30, 2025 · 1 comment · Fixed by #53
Closed

Tidy up sais.rs #48

ajalab opened this issue Jan 30, 2025 · 1 comment · Fixed by #53

Comments

@ajalab
Copy link
Owner

ajalab commented Jan 30, 2025

It's not obvious for readers that the module and function name sais computes a suffix array. Maybe it should be something like suffix_array instead.

Also, some functions in sais module like count_chars and get_bucket_start_pos are not specific to SA-IS algorithm nor suffix array. FMIndex::create uses these functions to build cs, but this misleads readers that the cs structure construction is related to SA-IS. Maybe these functions should reside in characters.rs or another module.

@faassen
Copy link
Collaborator

faassen commented Jan 30, 2025

Both sounds like good steps forward. I think cleaning up code is often a good way to start on something tricky, which SA-IS definitely is.

This was referenced Jan 31, 2025
@ajalab ajalab linked a pull request Feb 5, 2025 that will close this issue
@ajalab ajalab closed this as completed in #53 Feb 6, 2025
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 a pull request may close this issue.

2 participants