Add min exponent width option in double-to-string shortest conversion #116

pengzhenyi2015 · 2019-08-01T04:19:43Z

Prefix 0 is added when call golang fmt.Sprint() to convert double to shortest string
For example: 0.00001 is convert to 1e-05, but not 1e-5

In some situations, I want to convert double to shortest string in "golang format", so a "min exponent width option" will be useful

googlebot · 2019-08-01T04:19:46Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

pengzhenyi2015 · 2019-08-01T05:17:46Z

@googlebot I signed it!)

googlebot · 2019-08-01T05:17:54Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

floitsch

Please also update the Changelog.
Feel free to add yourself to the AUTHORS file as well.

floitsch · 2019-08-01T10:27:58Z

double-conversion/double-to-string.h

@@ -111,7 +111,8 @@ class DoubleToStringConverter {
                          int decimal_in_shortest_low,
                          int decimal_in_shortest_high,
                          int max_leading_padding_zeroes_in_precision_mode,
-                          int max_trailing_padding_zeroes_in_precision_mode)
+                          int max_trailing_padding_zeroes_in_precision_mode,
+                          int min_exponent_width = 0)


Also add a line (or two) to the comment of the constructor, explaining what the parameter does.
Shouldn't need a lot of explanation (quite self-explanatory), but mention that the exponent will never have more than 5 digits.
Then also add a comment to the constant (5) in double-to-string.cc and state that changing this constant also requires updating the comment in double-to-string.h

Thanks for your advice, I added some comments to introduce this option

floitsch · 2019-08-01T10:29:57Z

test/cctest/test-conversions.cc

+  builder.Reset();
+  CHECK(dcExpWidth.ToShortest(1000000000.0, &builder));
+  CHECK_EQ("1e+09", builder.Finalize());
+


Add a few more tests with corner-cases:

min_exponent_width set to 0. (which is the default, but still)

min_exponent_width set to 100.

The corner-cases are added

floitsch · 2019-08-01T10:32:09Z

Thanks.

pengzhenyi2015 · 2019-08-01T12:34:20Z

@floitsch Thanks for your advice

floitsch · 2019-08-01T12:36:30Z

double-conversion/double-to-string.h

@@ -104,14 +104,18 @@ class DoubleToStringConverter {
  //   ToPrecision(230.0, 2) -> "230"
  //   ToPrecision(230.0, 2) -> "230."  with EMIT_TRAILING_DECIMAL_POINT.
  //   ToPrecision(230.0, 2) -> "2.3e2" with EMIT_TRAILING_ZERO_AFTER_POINT.
+  //
+  // The exponent width may add up to min_exponent_width by adding prefix '0',
+  // but the exponent will never have more than 5 digits as kMaxExponentLength is 5


No need to mention kMaxExponentLength. It's an internal detail.
Simply:
"When min_exponent_width is used for exponential representations. The converter adds leading '0's, to the exponent until the exponent is at least min_exponent_width digits long.
The min_exponent_width is clamped to 5. As such, the exponent may never have more than 5 digits in total."

or something similar.

pengzhenyi2015 · 2019-08-01T12:51:02Z

The comment is updated

floitsch · 2019-08-01T13:57:44Z

double-conversion/double-to-string.h

@@ -104,14 +104,21 @@ class DoubleToStringConverter {
  //   ToPrecision(230.0, 2) -> "230"
  //   ToPrecision(230.0, 2) -> "230."  with EMIT_TRAILING_DECIMAL_POINT.
  //   ToPrecision(230.0, 2) -> "2.3e2" with EMIT_TRAILING_ZERO_AFTER_POINT.
+  //
+  // When min_exponent_width is used for exponential representations.


Suggested change

// When min_exponent_width is used for exponential representations.

// The min_exponent_width is used for exponential representations.

floitsch · 2019-08-01T13:58:39Z

One last tiny nit left.

floitsch · 2019-08-01T14:44:22Z

Thanks.

pengzhenyi2015 closed this Aug 1, 2019

pengzhenyi2015 reopened this Aug 1, 2019

floitsch approved these changes Aug 1, 2019

View reviewed changes

pengzhenyi2015 force-pushed the master branch from 1ce8615 to e4a9385 Compare August 1, 2019 12:27

floitsch reviewed Aug 1, 2019

View reviewed changes

pengzhenyi2015 force-pushed the master branch from e4a9385 to 1a77faf Compare August 1, 2019 12:48

floitsch reviewed Aug 1, 2019

View reviewed changes

pengzhenyi2015 closed this Aug 1, 2019

pengzhenyi2015 force-pushed the master branch from 1a77faf to 3f9cd30 Compare August 1, 2019 14:37

Add min exponent width option in double-to-string conversion

53be43a

pengzhenyi2015 reopened this Aug 1, 2019

floitsch merged commit 7da5db9 into google:master Aug 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add min exponent width option in double-to-string shortest conversion #116

Add min exponent width option in double-to-string shortest conversion #116

pengzhenyi2015 commented Aug 1, 2019

googlebot commented Aug 1, 2019

pengzhenyi2015 commented Aug 1, 2019

googlebot commented Aug 1, 2019

floitsch left a comment

floitsch Aug 1, 2019

pengzhenyi2015 Aug 1, 2019

floitsch Aug 1, 2019

pengzhenyi2015 Aug 1, 2019

floitsch commented Aug 1, 2019

pengzhenyi2015 commented Aug 1, 2019

floitsch Aug 1, 2019

floitsch Aug 1, 2019

pengzhenyi2015 commented Aug 1, 2019

floitsch Aug 1, 2019

floitsch commented Aug 1, 2019

floitsch commented Aug 1, 2019

	// When min_exponent_width is used for exponential representations.
	// The min_exponent_width is used for exponential representations.

Add min exponent width option in double-to-string shortest conversion #116

Add min exponent width option in double-to-string shortest conversion #116

Conversation

pengzhenyi2015 commented Aug 1, 2019

googlebot commented Aug 1, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

pengzhenyi2015 commented Aug 1, 2019

googlebot commented Aug 1, 2019

floitsch left a comment

Choose a reason for hiding this comment

floitsch Aug 1, 2019

Choose a reason for hiding this comment

pengzhenyi2015 Aug 1, 2019

Choose a reason for hiding this comment

floitsch Aug 1, 2019

Choose a reason for hiding this comment

pengzhenyi2015 Aug 1, 2019

Choose a reason for hiding this comment

floitsch commented Aug 1, 2019

pengzhenyi2015 commented Aug 1, 2019

floitsch Aug 1, 2019

Choose a reason for hiding this comment

floitsch Aug 1, 2019

Choose a reason for hiding this comment

pengzhenyi2015 commented Aug 1, 2019

floitsch Aug 1, 2019

Choose a reason for hiding this comment

floitsch commented Aug 1, 2019

floitsch commented Aug 1, 2019