Repository navigation
Remove the Build traits from bit_vectors and int_vectors - #129
Merged
Merged
Conversation
The `Build` traits duplicated each type's own constructors and hid type-specific behavior behind boolean flags whose meanings differed per implementation (some ignored, some rejected at runtime). The only generic user was `WaveletMatrix::new`. `WaveletMatrix::new` now takes a closure that builds each layer from a `BitVector`, e.g. `WaveletMatrix::new(seq, Rank9Sel::new)`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bit_vectors::Buildandint_vectors::Build, along with their impls andpreludere-exports.WaveletMatrix::newto take a closure that builds each layer from aBitVector.Why
The
Buildtraits were an over-abstraction:bit_vectors::Build::build_from_bits(bits, with_rank, with_select1, with_select0)took three positional bools whose meaning differed per type.BitVectorignored all of them,Rank9Selignoredwith_rank,DArrayignoredwith_select1, andSArrayreturned an error forwith_select0. Unsupported features were only detected at runtime.from_bits,enable_rank,select1_hints, ...).int_vectors::Buildhad no generic users at all, wrapped infallible constructors inResult, and hidDacsOpt'smax_levelsparameter.WaveletMatrix::new.New
WaveletMatrixAPIThe bench crate keeps the previous configurations (Rank9Sel with both select hints; DArray with
enable_rank().enable_select0()).Notes for reviewers
WaveletMatrix::newgains an argument. This should go into 0.10.0.SArraystill cannot be used as aWaveletMatrixlayer because it lacks select0. Previously this failed at construction with an error; now it panics whenselectis called. Thenewdocs state that layers must support both select1 and select0.SArray'stest_rs_build_with_s0was removed since the code path it tested no longer exists.Test plan
cargo fmt --all --checkcargo clippy --workspace --all-targets --all-features -- -D warnings -W clippy::nurserycargo clippy --all-targets --no-default-features -- -D warnings -W clippy::nurserycargo test/cargo test --release --no-default-featurescargo doc --no-deps🤖 Generated with Claude Code