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

formatPrefix will sometimes not perform rounding #132

Open
jdharrisnz opened this issue Oct 6, 2022 · 5 comments
Open

formatPrefix will sometimes not perform rounding #132

jdharrisnz opened this issue Oct 6, 2022 · 5 comments

Comments

@jdharrisnz
Copy link

For example: d3.formatPrefix(',.2', 1e3)(1555) produces 1.55, should be 1.56.

This is likely a simple fix, but unfortunately I'm not adept enough to figure out the source code and submit a PR, sorry.

@jdharrisnz
Copy link
Author

jdharrisnz commented Oct 6, 2022

This appears to be a problem with Chrome's .toFixed() function, where numbers ending in 5 won't round up. The workaround is to use Math.round() before calling .toFixed().

This seems to require an update to the f type in formatTypes.js.

So for the above case it would be:
(Math.round(1.555*100)/100).toFixed(2)

And in the generalised sense:
"f": (x, p) => (Math.round(x * Number('1e' + i)) / Number('1e' + i)).toFixed(p)

@Fil
Copy link
Member

Fil commented Oct 6, 2022

I'm seeing the same behavior with Firefox and Safari. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toFixed, which has a detailed explanation (for 2.55).

@mbostock
Copy link
Member

mbostock commented Oct 6, 2022

Working as intended.

@mbostock mbostock closed this as completed Oct 6, 2022
@mbostock
Copy link
Member

mbostock commented Oct 6, 2022

Well, (1.555).toFixed(2) producing "1.55" is the intended behavior. But I guess d3.formatPrefix introducing rounding error is not intended.

@mbostock mbostock reopened this Oct 6, 2022
@jdharrisnz
Copy link
Author

I made a local branch to submit a PR but I don't have write access.

The needed change is to formatTypes.js.

"f": (x, p) => (Math.round(x * Number('1e' + p)) / Number('1e' + p)).toFixed(p);

My earlier comment had i instead of p for some reason, oops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants