-
Notifications
You must be signed in to change notification settings - Fork 37
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 d3.pathRound. #12
Conversation
I’m pretty sure we don’t want to introduce the overhead of calling the identity function in the common case, which means a lot of code duplication. |
Now trying to call (I'll review the git pseudo-conflicts situation if the approach seems acceptable.) |
I think of format as something that takes a number and returns a string. Relying on format to return a number (and taking arbitrary input) suggests that “format” is the wrong name. Did you overwrite the original commits? |
Hmm it looks like I force-pushed over your original commit ( 7b1c410 ) in June 2019, after a rebase. |
should we name this function |
I’ve updated this to take a cleaner approach using an internal tagged template literal, and changed d3.pathFixed so that it defaults digits to three (which matches number.toLocaleString and seems like a very reasonable default). I think this is ready to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love the templating approach!
The latest commit switches to rounding per performance recommendations in #10 and d3/d3-geo#272 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🏇
Fixes #10, taking option 2.