Skip to content
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

[5.0] [Feature] support largest-triangle-three-buckets algo #13317

Merged
merged 2 commits into from
Sep 24, 2020
Merged

[5.0] [Feature] support largest-triangle-three-buckets algo #13317

merged 2 commits into from
Sep 24, 2020

Conversation

SnailSword
Copy link
Contributor

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Support largest-triangle-three-buckets sampling algorithm.

Fixed issues

#9403

Usage

Are there any API changes?

  • The API has been changed.

sampling: 'lttb'

Related test cases or examples to use the new APIs

{
  type: 'line',
  sampling: 'lttb',
  data: data
}

See test/line-large.html line 101.

Others

Merging options

  • Please squash the commits into a single one when merge.

Other information

In master branch it works, see #13312.
But in next branch, because of #12997 , samplers doesn't run in some cases.
And when it run, there is a point highlight bug, I think it's better to fix those bugs in other pr.

@echarts-bot
Copy link

echarts-bot bot commented Sep 21, 2020

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

The pull request is marked to be PR: author is committer because you are a committer of this project.

Document changes are required in this PR. Please also make a PR to apache/incubator-echarts-doc for document changes. When the doc PR is merged, the maintainers will remove the PR: awaiting doc label.

@echarts-bot echarts-bot bot added PR: author is committer PR: awaiting doc Document changes is required for this PR. PR: awaiting review labels Sep 21, 2020
@SnailSword SnailSword requested a review from pissang September 21, 2020 10:18
@pissang pissang changed the title feat(sample): support largest-triangle-three-buckets algo [5.0] [Feature] support largest-triangle-three-buckets algo Sep 22, 2020
/**
* Large data down sampling using largest-triangle-three-buckets
* copied from https://github.com/pingec/downsample-lttb with some modifications
* @param {string} baseDimension
Copy link
Contributor

@pissang pissang Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use the code directly. It will have a license issue.

If you are sure that the code is definitely different. You should avoid using terms like copy from, Referencing, etc. in the comment.

@pissang
Copy link
Contributor

pissang commented Sep 22, 2020

Also, do you mind adding a test case to compare the result and performance difference between lttb sampling enabled and disabled?

The example used in #13314 (comment) will be a good case.

And there are some code style like inconsistent indentation needs to be fixed.

@SnailSword
Copy link
Contributor Author

Also, do you mind adding a test case to compare the result and performance difference between lttb sampling enabled and disabled?

The example used in #13314 (comment) will be a good case.

demo

@pissang Thank you for your advise. I added comparison of results in test/sample-compare.html. But I don't know how to compare the performance, time spent on this part(the pic below) is almost the same in each downsampling method, and also same as that without downsampling. I don't think it is the correct way to compare the performance.

image

And there are some code style like inconsistent indentation needs to be fixed.

Fixed.

@echarts-bot
Copy link

echarts-bot bot commented Sep 24, 2020

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: author is committer PR: awaiting doc Document changes is required for this PR. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants