fix: change the calculation of precision in round method#21123
fix: change the calculation of precision in round method#21123SihongShen wants to merge 1 commit intoapache:masterfrom
Conversation
| export function round(x: number | string, precision?: number, returnStr?: boolean): string | number { | ||
| if (precision == null) { | ||
| precision = 10; | ||
| const expStr = (x + '').split('e')[1]; |
There was a problem hiding this comment.
Computing precision relying solely on scientific notation is not reliable enough.
e.g. , 0.0000010000000001 + '' => 0.0000010000000001 + '', not scientific notation, even if it's fractional digits are over 10.
There is a method getPrecision in this file that provides the precision calculation.
But I'm afraid it doesn't seems reasonable to "round based on the precision of a number" - the result appear to be that nothing happens.
This method is meant to round the input number with a given precision. The default value 10, by historical reasons, is an arbitrary value, and perhaps it shouldn't be provided - the caller should always provide a precision based on the scenario, and there is no universally adaptable default precision.
The bad case (as posted in #16266) results from the calling of this method from https://github.com/apache/echarts/blob/5.6.0/src/scale/Interval.ts#L284 uses the default precision 10.
Actually, I think a this._intervalPrecision should be used here to fix this bug.
A quick fix: #21126 to hitch a ride on this v6.
Brief Information
This pull request is in the type of:
What does this PR do?
Revise the calculation of precision in round function based on PR #16286 .
Fixed issues
#16266
Details
Before: What was the problem?
The dataset ['0.000000000012', '0.000000000034', '0.000000000010', '0.000000000038'] was round to 0 as data are too small.
After: How does it behave after the fixing?
The dataset can be displayed normally after changing the precision based on the review suggestions in PR #16286. The testing examples is added to /test/logScale.html .
Document Info
One of the following should be checked.
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information