Skip to content

Conversation

@leerho
Copy link
Contributor

@leerho leerho commented Oct 16, 2025

This pretty much completes the change of class names that were either in conflict with other sketch families or could cause confusion. For example, the "LongsSketch" in FrequentItems is now "FrequentLongsSketch", etc. I also changed the names of some enum constants to be consistent.

leerho added 6 commits October 6, 2025 17:31
This only involved mostly public classes.
Note that I focused on changing the name of public classes where they
might conflict with similarly named classes in other families,
particularly the /theta/ and /tuple/ families.
In KLL, I changed the names of the enum constants to KLL_DOUBLES_SKETCH,
etc.  And the methods to isKllItmesSketch(), etc.
@leerho leerho marked this pull request as ready for review October 16, 2025 20:44
@leerho
Copy link
Contributor Author

leerho commented Oct 16, 2025

I have completed, I think, all the necessary name changes for /theta/ and /tuple/ -- 124 files,
And /kll/ and /frequencies/ -- another 38 files.

I primarily focused on the public classes and methods where:

  • There was an overlap of class names: This occured mostly between:
    • /theta/ and /tuple/
    • /kll/, /frequencies/, /quantiles/.
    • By fixing the names in KLL and FI, I didn't have to change the names in classic quantiles.
  • It provided additional clarity.
  • I did not attempt to rename the many public classes where the name was already unique and it was not duplicated in any other family. I felt that those were easy to understand what family they belonged to by their context.

Bear in mind that there are no code logic changes whatsoever. But of course, the name changes constitute breaking changes on their own. All the unit tests run just fine, even the CI tests.

Nonetheless, CoPilot complained about the fact that the deprecated methods are still used in test, and still have a few transient references in /main/ that a are unavoidable — unless we remove all of the deprecated methods, which I don’t have the energy or desire to do for this release.

CoPilot also complained about unused variables in the tests -- which I mostly fixed - although I'm not too concerned about this in test code.

If I can get this quickly approved and merged by Oct 18th, I might -- but no guarantees -- have the time to do a release before I leave on a long vacation. Otherwise, it will have to wait until the 2nd week in November.

@leerho leerho merged commit 4d1b5b7 into main Oct 17, 2025
7 checks passed
@leerho leerho deleted the cleanup_names_phase_5 branch October 17, 2025 20:55
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