refactor: align seed and internal naming - #164
Merged
Merged
Conversation
Signed-off-by: tison <wander4096@gmail.com>
tisonkun
marked this pull request as ready for review
July 25, 2026 14:14
Member
Author
|
cc @ariesdevil in case you're working on theta/tuple sketches as well. This can be a code/logical conflict. |
tisonkun
enabled auto-merge (squash)
July 25, 2026 14:14
ZENOTME
approved these changes
Jul 25, 2026
Contributor
|
Thx for informing me:) |
3 tasks
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
This PR applies the approved subset of the naming-drift audit. It establishes an explicit distinction between a full 64-bit
seedand its derived 16-bitseed_hash, standardizes the set-operation constructor style, and corrects several misleading or non-idiomatic internal names.Breaking constructor changes
Theta and Tuple set-operation constructors now follow the same convention as the other sketches:
ThetaIntersection::default()uses the default seed;ThetaIntersection::with_seed(seed)accepts a custom seed.ThetaAnotB::default()uses the default seed;ThetaAnotB::with_seed(seed)accepts a custom seed.TupleIntersection::new(policy)uses the default seed;TupleIntersection::with_seed(policy, seed)accepts a custom seed.TupleAnotB::default()uses the default seed;TupleAnotB::with_seed(seed)accepts a custom seed.The previous
new(seed)andnew_with_default_seedforms are removed and the migration is recorded inCHANGELOG.md.Commit-by-commit rationale
7f15f37— S1: define seed terminology. Documents thatseedis the full 64-bit hashing value,seed_hashis only the derived 16-bit compatibility fingerprint, andhash_seedmust not be used as a synonym.643a358— S2: rename the full RawHashTable seed. Changes the internalhash_seedfield, parameters, and accessor toseed, removing the only third synonym while leaving Count-Min rowhash_seedsintact.0646322— S3: standardize seed-hash errors. Converts the remaining CPC and A-not-B outliers to the commonincompatible seed hashwording while retaining A/B input context.1278d7b— C1: standardize set-operation constructors. Introduces consistent default-seed and custom-seed entry points across Theta and Tuple intersection and A-not-B operators; updates examples, tests, and the changelog.d7db047— P1: align CPC confidence parameter names. Usesnum_std_devon public CPC sketch and wrapper methods while retaining the mathematicalkappaname inside estimator internals.d073774— P3: align ThetaUnionBuilder parameter names. Renames local parameters tofactorandprobability, matching the equivalent Theta and Tuple builders without changing method names or behavior.9611f37— P4: clarify the raw A-not-B type. Renames the shared internal operator fromRawAnotBtoRawThetaAnotB, making its relationship toRawThetaUnionandRawThetaIntersectionexplicit.2a01ef3— I2: name the hash-table threshold accurately. Replacesget_capacitywithcapacity_threshold, since the value controls resize or rebuild rather than reporting allocated vector capacity.01914e5— I3: use the approved entry lookup name. Replacesget_entrywithentry, following the selected Rust-style accessor name.d45955c— I4: expose intersection state directly. Renames the internalis_validfield tohas_result, matching the actual state-machine meaning and the existing public method.b0d38fb— I5: use the plural byte-count name. Renamesbyte_usedtobytes_usedand corrects its documentation because the method returns a count.b8d7242— I6: follow mutable-accessor convention. Renamesmut_surprising_value_tabletosurprising_value_table_mut, matching standard Rust_mutsuffix usage.76a78b9— I7: correct the raw-view documentation. Describes the view as shared by Theta-family union, intersection, and A-not-B operations rather than union alone.1264c3a— Use Default for zero-configuration operators. Replaces zero-argumentnew()methods withDefaultimplementations onThetaIntersection,ThetaAnotB, andTupleAnotB, matching the existing CPC, T-Digest, and builder conventions.TupleIntersection::new(policy)remains because its policy is required.Validation
cargo x testcargo x lintmainafter chore: simplify test support code #163Explicitly deferred
This PR does not change the
RawThetaSketchView::thetaoriterAPI, does not remove cachedseed_hashfields, does not renameCoupon::from_hash, and does not include any other items from the audit that were not explicitly approved.