-
Notifications
You must be signed in to change notification settings - Fork 697
MB-66396: Index APIs for TermFrequencies and CentroidCardinalities
#2207
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
base: master
Are you sure you want to change the base?
Conversation
87e97aa to
9a7378d
Compare
HighestFrequencyTerms and HighestCardinalityCentroids
HighestFrequencyTerms and HighestCardinalityCentroidsTermFrequencies and CentroidCardinalities
| return nil | ||
| } | ||
|
|
||
| func (i *IndexSnapshot) CentroidCardinalities(field string, limit int, descending bool) ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this function is in snapshot_index_vr.go, the method wouldn't be included in IndexSnapshot if the vectors flag is turned off. So it wouldn't be an implementation of IndexInsightsReader anymore. Calls to TermFrequencies in index_impl.go would then fail because the code checks if the interface is implemented for the reader.
Basically, TermFrequencies would only work if it is compiled with vectors, is this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya it was intentional - because the insights project is meant for AI applications which will use this alongside vector data. This is just a start though, hoping to add more functions to the interface.
|
Thanks @CascadingRadium for the review so far, will push up a patch reflecting on your comments soon. |
0be6e44 to
cc615ca
Compare
Requires:
IndexInsightsReaderbleve_index_api#71