| Message ID | 1335558083-26196-5-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 27.04.2012 22:21, schrieb Michael Roth: > Currently string-output-visitor formats floats as %g, which is nice in > that trailing 0's are automatically truncated, but otherwise this causes > some issues: > > - it 6 uses significant figures instead of 6 decimal places, which "it uses 6 significant" > means something like 155777.5 (which even has an exact floating point > representation) will be rounded to 155778 when converted to a string. > > - output will be presented in scientific notation when the normalized > form requires a 10^x multiplier. Not a huge deal, but arguably less > readable for command-line arguments. > > - due to using sig figs instead of hard-defined decimal places, it "six figs"? > fails a lot of the test-visitor-serialization unit tests for floats. > > Instead, let's just use %f, which is what the QJSON and the QMP visitors > use. > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> /-F
On Sat, Apr 28, 2012 at 06:20:47PM +0200, Andreas Färber wrote: > Am 27.04.2012 22:21, schrieb Michael Roth: > > Currently string-output-visitor formats floats as %g, which is nice in > > that trailing 0's are automatically truncated, but otherwise this causes > > some issues: > > > > - it 6 uses significant figures instead of 6 decimal places, which > > "it uses 6 significant" > Doh, I'll send an updated patch with the commit msg fixed. > > means something like 155777.5 (which even has an exact floating point > > representation) will be rounded to 155778 when converted to a string. > > > > - output will be presented in scientific notation when the normalized > > form requires a 10^x multiplier. Not a huge deal, but arguably less > > readable for command-line arguments. > > > > - due to using sig figs instead of hard-defined decimal places, it > > "six figs"? > significant figures, "scientific notation" is probably clearer, I'll include it in the update. > > fails a lot of the test-visitor-serialization unit tests for floats. > > > > Instead, let's just use %f, which is what the QJSON and the QMP visitors > > use. > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > /-F > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
On Fri, 27 Apr 2012 15:21:20 -0500 Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > Currently string-output-visitor formats floats as %g, which is nice in > that trailing 0's are automatically truncated, but otherwise this causes > some issues: > > - it 6 uses significant figures instead of 6 decimal places, which > means something like 155777.5 (which even has an exact floating point > representation) will be rounded to 155778 when converted to a string. > > - output will be presented in scientific notation when the normalized > form requires a 10^x multiplier. Not a huge deal, but arguably less > readable for command-line arguments. > > - due to using sig figs instead of hard-defined decimal places, it > fails a lot of the test-visitor-serialization unit tests for floats. > > Instead, let's just use %f, which is what the QJSON and the QMP visitors > use. > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > qapi/string-output-visitor.c | 2 +- > tests/test-string-output-visitor.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c > index 92b0305..34e525e 100644 > --- a/qapi/string-output-visitor.c > +++ b/qapi/string-output-visitor.c > @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, const char *name, > Error **errp) > { > StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); > - string_output_set(sov, g_strdup_printf("%g", *obj)); > + string_output_set(sov, g_strdup_printf("%f", *obj)); Doesn't look like a bug fix worth it for 1.1, am I wrong? > } > > char *string_output_get_string(StringOutputVisitor *sov) > diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c > index 22909b8..608f14a 100644 > --- a/tests/test-string-output-visitor.c > +++ b/tests/test-string-output-visitor.c > @@ -84,7 +84,7 @@ static void test_visitor_out_number(TestOutputVisitorData *data, > > str = string_output_get_string(data->sov); > g_assert(str != NULL); > - g_assert_cmpstr(str, ==, "3.14"); > + g_assert_cmpstr(str, ==, "3.140000"); > g_free(str); > } >
On Fri, May 11, 2012 at 01:34:01PM -0300, Luiz Capitulino wrote: > On Fri, 27 Apr 2012 15:21:20 -0500 > Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > > > Currently string-output-visitor formats floats as %g, which is nice in > > that trailing 0's are automatically truncated, but otherwise this causes > > some issues: > > > > - it 6 uses significant figures instead of 6 decimal places, which > > means something like 155777.5 (which even has an exact floating point > > representation) will be rounded to 155778 when converted to a string. > > > > - output will be presented in scientific notation when the normalized > > form requires a 10^x multiplier. Not a huge deal, but arguably less > > readable for command-line arguments. > > > > - due to using sig figs instead of hard-defined decimal places, it > > fails a lot of the test-visitor-serialization unit tests for floats. > > > > Instead, let's just use %f, which is what the QJSON and the QMP visitors > > use. > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > --- > > qapi/string-output-visitor.c | 2 +- > > tests/test-string-output-visitor.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c > > index 92b0305..34e525e 100644 > > --- a/qapi/string-output-visitor.c > > +++ b/qapi/string-output-visitor.c > > @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, const char *name, > > Error **errp) > > { > > StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); > > - string_output_set(sov, g_strdup_printf("%g", *obj)); > > + string_output_set(sov, g_strdup_printf("%f", *obj)); > > Doesn't look like a bug fix worth it for 1.1, am I wrong? Well, object_property_print() is the only string-output-visitor user, and it's not currently used. I don't see this changing for 1.1, so this can probably wait. > > > } > > > > char *string_output_get_string(StringOutputVisitor *sov) > > diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c > > index 22909b8..608f14a 100644 > > --- a/tests/test-string-output-visitor.c > > +++ b/tests/test-string-output-visitor.c > > @@ -84,7 +84,7 @@ static void test_visitor_out_number(TestOutputVisitorData *data, > > > > str = string_output_get_string(data->sov); > > g_assert(str != NULL); > > - g_assert_cmpstr(str, ==, "3.14"); > > + g_assert_cmpstr(str, ==, "3.140000"); > > g_free(str); > > } > > >
Am 11.05.2012 19:32, schrieb Michael Roth: > On Fri, May 11, 2012 at 01:34:01PM -0300, Luiz Capitulino wrote: >> On Fri, 27 Apr 2012 15:21:20 -0500 >> Michael Roth <mdroth@linux.vnet.ibm.com> wrote: >> >>> Currently string-output-visitor formats floats as %g, which is nice in >>> that trailing 0's are automatically truncated, but otherwise this causes >>> some issues: >>> >>> - it 6 uses significant figures instead of 6 decimal places, which >>> means something like 155777.5 (which even has an exact floating point >>> representation) will be rounded to 155778 when converted to a string. >>> >>> - output will be presented in scientific notation when the normalized >>> form requires a 10^x multiplier. Not a huge deal, but arguably less >>> readable for command-line arguments. >>> >>> - due to using sig figs instead of hard-defined decimal places, it >>> fails a lot of the test-visitor-serialization unit tests for floats. >>> >>> Instead, let's just use %f, which is what the QJSON and the QMP visitors >>> use. >>> >>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> >>> --- >>> qapi/string-output-visitor.c | 2 +- >>> tests/test-string-output-visitor.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c >>> index 92b0305..34e525e 100644 >>> --- a/qapi/string-output-visitor.c >>> +++ b/qapi/string-output-visitor.c >>> @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, const char *name, >>> Error **errp) >>> { >>> StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); >>> - string_output_set(sov, g_strdup_printf("%g", *obj)); >>> + string_output_set(sov, g_strdup_printf("%f", *obj)); >> >> Doesn't look like a bug fix worth it for 1.1, am I wrong? > > Well, object_property_print() is the only string-output-visitor user, > and it's not currently used. I don't see this changing for 1.1, so this > can probably wait. Actually it might be in 1.1: there's a pending patch by Paolo to use that for info qtree, where some properties were missing. My review comment has been resolved, so I will queue that patch for 1.1 and next. Andreas
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index 92b0305..34e525e 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, const char *name, Error **errp) { StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); - string_output_set(sov, g_strdup_printf("%g", *obj)); + string_output_set(sov, g_strdup_printf("%f", *obj)); } char *string_output_get_string(StringOutputVisitor *sov) diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c index 22909b8..608f14a 100644 --- a/tests/test-string-output-visitor.c +++ b/tests/test-string-output-visitor.c @@ -84,7 +84,7 @@ static void test_visitor_out_number(TestOutputVisitorData *data, str = string_output_get_string(data->sov); g_assert(str != NULL); - g_assert_cmpstr(str, ==, "3.14"); + g_assert_cmpstr(str, ==, "3.140000"); g_free(str); }
Currently string-output-visitor formats floats as %g, which is nice in that trailing 0's are automatically truncated, but otherwise this causes some issues: - it 6 uses significant figures instead of 6 decimal places, which means something like 155777.5 (which even has an exact floating point representation) will be rounded to 155778 when converted to a string. - output will be presented in scientific notation when the normalized form requires a 10^x multiplier. Not a huge deal, but arguably less readable for command-line arguments. - due to using sig figs instead of hard-defined decimal places, it fails a lot of the test-visitor-serialization unit tests for floats. Instead, let's just use %f, which is what the QJSON and the QMP visitors use. Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qapi/string-output-visitor.c | 2 +- tests/test-string-output-visitor.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)