You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2021/12/27 08:25:36 UTC
[GitHub] [echarts] songlinn opened a new pull request #16286: fix: change the precision of number to Number.toFixed max precision
songlinn opened a new pull request #16286:
URL: https://github.com/apache/echarts/pull/16286
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
fix number default precision
### Fixed issues
#16266
## Details
### Before: What was the problem?
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
### After: How is it fixed in this PR?
At issues, the dataset is `['0.000000000012', '0.000000000034', '0.000000000010', '0.000000000038']`, but the number default precision is 10 ,value " 0.000000000012" will be round to 0. therefore i change the default precision to the constant value ROUND_SUPPORTED_PRECISION_MAX.
## Misc
<!-- ADD RELATED ISSUE ID WHEN APPLICABLE -->
- [ ] The API has been changed (apache/echarts-doc#xxx).
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
N.A.
## Others
### Merging options
- [ ] Please squash the commits into a single one when merging.
### Other information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] songlinn commented on a change in pull request #16286: fix: change the precision of number to Number.toFixed max precision
Posted by GitBox <gi...@apache.org>.
songlinn commented on a change in pull request #16286:
URL: https://github.com/apache/echarts/pull/16286#discussion_r775477608
##########
File path: src/util/number.ts
##########
@@ -138,7 +138,7 @@ export function round(x: number | string, precision: number, returnStr: false):
export function round(x: number | string, precision: number, returnStr: true): string;
export function round(x: number | string, precision?: number, returnStr?: boolean): string | number {
if (precision == null) {
- precision = 10;
+ precision = ROUND_SUPPORTED_PRECISION_MAX;
Review comment:
What do you think auto calculate precision by number decimal place length?
such as :
``` js
if (precision == null) {
const exp = String(x).split('e')[1];
precision = Number(-exp) || 10;
}
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16286: fix: change the precision of number to Number.toFixed max precision
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16286:
URL: https://github.com/apache/echarts/pull/16286#discussion_r775786860
##########
File path: src/util/number.ts
##########
@@ -138,7 +138,9 @@ export function round(x: number | string, precision: number, returnStr: false):
export function round(x: number | string, precision: number, returnStr: true): string;
export function round(x: number | string, precision?: number, returnStr?: boolean): string | number {
if (precision == null) {
- precision = 10;
+ // auto adjust round precision by decimal place length only when the number is using scientific
+ const exp = String(x).split('e')[1];
Review comment:
It's not the precision `x`. `1.3e-10` will have exp 10 here. But it should be 11.
Also we prefer x + '' to convert a value to string. And `-exp` already convert it to a number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #16286: fix: change the precision of number to Number.toFixed max precision
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #16286:
URL: https://github.com/apache/echarts/pull/16286#discussion_r775404396
##########
File path: src/util/number.ts
##########
@@ -138,7 +138,7 @@ export function round(x: number | string, precision: number, returnStr: false):
export function round(x: number | string, precision: number, returnStr: true): string;
export function round(x: number | string, precision?: number, returnStr?: boolean): string | number {
if (precision == null) {
- precision = 10;
+ precision = ROUND_SUPPORTED_PRECISION_MAX;
Review comment:
`ROUND_SUPPORTED_PRECISION_MAX` is a safe protection for `toFixed` methods. The actual precision issue in the JS engine is less than this.
Check this in your console
```ts
(0.1 + 0.2).toFixed(20);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] songlinn commented on a change in pull request #16286: fix: change the precision of number to Number.toFixed max precision
Posted by GitBox <gi...@apache.org>.
songlinn commented on a change in pull request #16286:
URL: https://github.com/apache/echarts/pull/16286#discussion_r775477608
##########
File path: src/util/number.ts
##########
@@ -138,7 +138,7 @@ export function round(x: number | string, precision: number, returnStr: false):
export function round(x: number | string, precision: number, returnStr: true): string;
export function round(x: number | string, precision?: number, returnStr?: boolean): string | number {
if (precision == null) {
- precision = 10;
+ precision = ROUND_SUPPORTED_PRECISION_MAX;
Review comment:
What do you think auto calculate precision by number decimal place length?
such as :
if (precision == null) {
const exp = String(x).split('e')[1];
precision = Number(-exp) || 10;
}`
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org