-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Add aggregation function for Largest-Triangle-Three-Buckets #53145
Conversation
This is an automated comment for commit b3b2277 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
@nickitat updated code according to report. |
@nickitat can we test this ? |
@nickitat I can see that it requires a |
Hey @nickitat , it appears that the code is passing all tests except for one unrelated test case in the performance comparison. Could we review the code? |
It'll probably pass if we retest this specific test https://github.com/ClickHouse/ClickHouse/actions/runs/5828437103/job/15815152543 . |
@nickitat @alexey-milovidov , please review the changes |
Hi @nickitat , it seems to be passing most of the test cases, failing ones doesn't seem to be related. |
Hi @nickitat , @alexey-milovidov , can we merge this ? |
@nickitat is on vacation this week, he will review later. |
|
Hi @nickitat , @alexey-milovidov , Just checking in on this PR. If any changes are needed, please let me know. Appreciate your time! |
IMO, it would be nice to have
https://docs.timescale.com/api/latest/hyperfunctions/downsampling/#downsampling-functions |
let's add the alias UnamedRus suggested and merge |
@nickitat added alias, you can proceed to merge |
|
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added aggregation function lttb which uses the Largest-Triangle-Three-Buckets algorithm for downsampling data for visualization.
Documentation entry for user-facing changes