Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation #6

cabeaulac · 2014-10-06T17:51:08Z

Changed include to use quotes instead of angle brackets.
This allows the include to be found in the local include path.

Changed include to use quotes instead of angle brackets. This allows the include to be found in the local include path.

The Python json module does not like scientific notation when loading a string using the json.loads() method.

cabeaulac · 2014-10-08T22:38:44Z

I just added a commit to this pull request. Changed %g to %f when using scanf to convert double into a string. %g creates doubles in scientific notation for large values and Python.json.loads(my_json) does not play nicely with scientific notation.

LB-- · 2014-10-09T00:02:23Z

The JSON standard explicitly allows for scientific notation.

Also it's not a good idea to combine two separate things into one pull request.

cabeaulac · 2014-10-09T03:10:06Z

It does but Python.json isn't parsing it correctly. What shall we do? Change to %f so the Python binding works or maybe there's a way to tell the Python.json.loads function to take scientific notation that I don't know about yet. What do you think?

On Oct 8, 2014, at 5:02 PM, LB-- [email protected] wrote:

The JSON standard explicitly allows for scientific notation.

—
Reply to this email directly or view it on GitHub.

jamesamcl · 2014-10-09T09:16:38Z

Could make it an option?

cabeaulac · 2014-10-09T14:29:23Z

It's changed in my version to %f so the JSON coming from an embedded C
application to Python will parse with the Python.json module. If you want
to make it an option, that sounds great to me. I don't know how you'd want
to do that. If you want to do it, I'll pull down the latest after you push.
Otherwise, you can tell me how you'd like it done and I'll add it to my
pull request.

Thanks,
Chad

On Thu, Oct 9, 2014 at 2:16 AM, James McLaughlin [email protected]
wrote:

Could make it an option?

—
Reply to this email directly or view it on GitHub
#6 (comment).

LB-- · 2014-10-09T16:45:35Z

Probably by adding an option to json-builder.h#L113-L145

#define json_serialize_opt_no_scientific_notation  (1 << 6)

And then changing the implementation to check for that option.

json-builder.h Added the json_serialize_opt_no_scientific_notation option. This changes the serialization code to use %f instead of %g when it's set.

cabeaulac · 2014-10-09T21:03:13Z

I pushed a change to implement the json_serialize_opt_no_scientific_notation. I'm running with it in my code and in my CUnit tests now. It's working good so far.

LB-- · 2014-10-09T22:05:11Z

json-builder.c

+              total += snprintf (NULL, 0, "%g", value->u.dbl);             
+           }
+           if (value->u.dbl - floor (value->u.dbl) < 0.001)
+              total += 2;


I think these lines are 1 space too shallow - James uses 3-space indents.

I didn't change the 2 space indent code. That was already there.

Sent from my iPhone

On Oct 9, 2014, at 3:05 PM, LB-- [email protected] wrote:

In json-builder.c:

@@ -663,11 +670,16 @@ size_t json_measure_ex (json_value * value, json_serialize_opts opts)
break;

case json_double:

- total += snprintf (NULL, 0, "%g", value->u.dbl);

if (value->u.dbl - floor (value->u.dbl) < 0.001)

total += 2;

if (no_scientific_notation)

{

total += snprintf (NULL, 0, "%f", value->u.dbl);

}

else

{

total += snprintf (NULL, 0, "%g", value->u.dbl);

}

if (value->u.dbl - floor (value->u.dbl) < 0.001)

total += 2;
I think these lines are 1 space too shallow - James uses 3-space indents.

—
Reply to this email directly or view it on GitHub.

Compare the original (deleted) lines 669 and 670 to your new 681 and 682

cabeaulac · 2014-10-10T15:26:23Z

Hmm. I'm not seeing it. Please elaborate. I'm brand new to this codebase and can be easily missing something.
Original

 if (value->u.dbl - floor (value->u.dbl) < 0.001)
                total += 2;

New

if (value->u.dbl - floor (value->u.dbl) < 0.001)
              total += 2;

jamesamcl · 2014-10-10T15:48:23Z

I think this would be more concise as a ternary if on the format anyway?

---- Chad Beaulac wrote ----

Hmm. I'm not seeing it. Please elaborate. I'm brand new to this codebase and can be easily missing something.
Original

if (value->u.dbl - floor (value->u.dbl) < 0.001) total += 2;

New

if (value->u.dbl - floor (value->u.dbl) < 0.001) total += 2;

—
Reply to this email directly or view it on GitHub.

LB-- · 2014-10-10T16:07:41Z

Hmm. I'm not seeing it.

I don't know how strict @udp is on formatting, I just know he prefers 3 spaces per indent.

cabeaulac · 2014-10-10T20:01:24Z

Ternary operator sounds good.

Sent from my iPhone

On Oct 10, 2014, at 8:48 AM, James McLaughlin [email protected] wrote:

I think this would be more concise as a ternary if on the format anyway?

---- Chad Beaulac wrote ----

Hmm. I'm not seeing it. Please elaborate. I'm brand new to this codebase and can be easily missing something.
Original

if (value->u.dbl - floor (value->u.dbl) < 0.001) total += 2;

New

if (value->u.dbl - floor (value->u.dbl) < 0.001) total += 2;

—
Reply to this email directly or view it on GitHub.
—
Reply to this email directly or view it on GitHub.

cabeaulac · 2014-10-10T20:27:54Z

Ok. It seems like there's definitely something wrong. Some code is returning shorter values for json_measure_ex than the string returned by json_serialize_ex. I'm writing unit tests to address it now.

cabeaulac · 2014-10-10T22:47:50Z

My Python unittest is working with my scientific notation. So, I believe the issue I was originally having is due to json_measure and json_serialize returning different length strings. I'll figure it out and post resolution. I'll probably back out the json_serialize_opt_no_scientific_notation option since it appears that wasn't the issue.

This reverts commit 49b2f2d.

This reverts commit c312e79.

cabeaulac · 2014-10-10T23:09:35Z

In json_serialize_ex, can somebody tell me the purpose of adding two spaces to the total if the subtraction condition is met? Thanks

         case json_double:

            total += snprintf (NULL, 0, "%g", value->u.dbl);

            if (value->u.dbl - floor (value->u.dbl) < 0.001)
                total += 2;

jamesamcl · 2014-10-11T09:32:22Z

It was added in 4885ca3. It's to allow room for serialize adding .0 when integer double values get serialized without a fractional part (which it does to make sure they're read back as doubles, rather than integers).

cabeaulac · 2014-10-11T15:17:09Z

Oh. Ok. That makes total sense. I'll write some CUnit tests that target
that area of the code. The json_measure function is returning a value 2
characters longer than necessary sometimes. I'm trying to figure out why.
It's only when using floats. And, two extra zeros at the end of a byte
array makes the Python json.loads(my_doc) raise and exception.

On Sat, Oct 11, 2014 at 2:32 AM, James McLaughlin [email protected]
wrote:

It was added in 4885ca3
4885ca3.
It's to allow room for serialize adding .0 when integer double values get
serialized without a fractional part (which it does to make sure they're
read back as doubles, rather than integers).

—
Reply to this email directly or view it on GitHub
#6 (comment).

LB-- · 2014-10-11T18:05:39Z

%g doesn't seem to work as expected: http://ideone.com/Z3wT6A

I think it would take less effort to just try both %f and %e manually and choose one instead of using %g and then trying to manually append .0

http://en.cppreference.com/w/cpp/io/c/fprintf

Unless alternative representation is requested the trailing zeros are removed, also the decimal point character is removed if no fractional part is left.

I don't know what it means by "alternative representation".

cabeaulac · 2014-10-13T16:04:23Z

How about this?

We use %f by default
Let user pass in the %f format string. Like "8.2". If the user provides a string that says x.0, we change the zero to a one.
Let client specify %e as an option. Format string is not used if this option is specified.

json-builder.h

cf06a8e

Changed include to use quotes instead of angle brackets. This allows the include to be found in the local include path.

cabeaulac changed the title ~~json-builder.h~~ Change include of json.h in json-builder.h Oct 6, 2014

Change %g to %f to scanf doubles when building JSON doc.

c312e79

The Python json module does not like scientific notation when loading a string using the json.loads() method.

cabeaulac changed the title ~~Change include of json.h in json-builder.h~~ Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation Oct 8, 2014

json-builder.c

49b2f2d

json-builder.h Added the json_serialize_opt_no_scientific_notation option. This changes the serialization code to use %f instead of %g when it's set.

LB-- reviewed Oct 9, 2014
View reviewed changes

cabeaulac added 3 commits October 10, 2014 15:56

Revert "json-builder.c"

7d9c5d2

This reverts commit 49b2f2d.

Revert "Change %g to %f to scanf doubles when building JSON doc."

0b71209

This reverts commit c312e79.

Ignore NetBeans project and revert two commits

776639f

LB-- mentioned this pull request Aug 10, 2021

Lost precision in double values #11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation #6

Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation #6

cabeaulac commented Oct 6, 2014

cabeaulac commented Oct 8, 2014

LB-- commented Oct 9, 2014

cabeaulac commented Oct 9, 2014

jamesamcl commented Oct 9, 2014

cabeaulac commented Oct 9, 2014

LB-- commented Oct 9, 2014

cabeaulac commented Oct 9, 2014

LB-- Oct 9, 2014

cabeaulac Oct 10, 2014

LB-- Oct 10, 2014

cabeaulac commented Oct 10, 2014

jamesamcl commented Oct 10, 2014

LB-- commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

jamesamcl commented Oct 11, 2014

cabeaulac commented Oct 11, 2014

LB-- commented Oct 11, 2014

cabeaulac commented Oct 13, 2014

Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation #6

Are you sure you want to change the base?

Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation #6

Conversation

cabeaulac commented Oct 6, 2014

cabeaulac commented Oct 8, 2014

LB-- commented Oct 9, 2014

cabeaulac commented Oct 9, 2014

jamesamcl commented Oct 9, 2014

cabeaulac commented Oct 9, 2014

LB-- commented Oct 9, 2014

cabeaulac commented Oct 9, 2014

LB-- Oct 9, 2014

Choose a reason for hiding this comment

cabeaulac Oct 10, 2014

Choose a reason for hiding this comment

- total += snprintf (NULL, 0, "%g", value->u.dbl);

LB-- Oct 10, 2014

Choose a reason for hiding this comment

cabeaulac commented Oct 10, 2014

jamesamcl commented Oct 10, 2014

LB-- commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

cabeaulac commented Oct 10, 2014

jamesamcl commented Oct 11, 2014

cabeaulac commented Oct 11, 2014

LB-- commented Oct 11, 2014

cabeaulac commented Oct 13, 2014