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.
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
[ML] add new bucket_correlation aggregation with initial count_correlation function #72133
[ML] add new bucket_correlation aggregation with initial count_correlation function #72133
Changes from 2 commits
5216280
f49c647
35dc158
62c9faf
ed37bf8
b404e9c
c3a06fb
5b4b30c
bdccff1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
As above.
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.
We should make this a fully real example, i think. It'd be a pain to make the setup for it, but without that we can't be sure it works.
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.
@nik9000 there is a non-doc integration test that covers this.
I can attempt to do a set up, but its gonna take a bit to generate data and write out the 50 ranges.
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.
Its important that we test these so they don't break eventually. I can't tell you the number of times I've broken stuff in the docs without noticing it. I mean, since we made the tests I can tell you its much rarer, but I still can't tell you.
You can totally use the
setup
stuff indocs/build.gradle
- over there you can write for loops and stuff to emit values.If we have a response on the page we should assert that it came from a request on the page - but its totally ok to use stuff like
...
andfilter_path
to shrink it. No one is going to read a huge response anyway.....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.
I think we should also have an example response. It seems to be present for most other aggs.
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.
See below :)
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.
Lots of files are changed in this PR because of the refactoring of pulling those into the super class. I would really appreciate splitting this refactoring into its own PR that can be merged before this one does. It would help keep this one clean and readable.