Change include of json.h in json-builder.h. Change %g to %f because Python.json doesn't like scientific notation by cabeaulac · Pull Request #6 · json-parser/json-builder

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-- notifications@github.com 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 notifications@github.com
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

#definejson_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

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-- notifications@github.com 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 notifications@github.com 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 notifications@github.com
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.hOct 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 notationOct 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

Uh oh!

Conversation

cabeaulac commented Oct 6, 2014

Uh oh!

cabeaulac commented Oct 8, 2014

Uh oh!

LB-- commented Oct 9, 2014

Uh oh!

cabeaulac commented Oct 9, 2014

Uh oh!

jamesamcl commented Oct 9, 2014

Uh oh!

cabeaulac commented Oct 9, 2014

Uh oh!

LB-- commented Oct 9, 2014

Uh oh!

cabeaulac commented Oct 9, 2014

Uh oh!

LB--Oct 9, 2014

Choose a reason for hiding this comment

Uh oh!

cabeaulacOct 10, 2014

Choose a reason for hiding this comment

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

Uh oh!

LB--Oct 10, 2014

Choose a reason for hiding this comment

Uh oh!

cabeaulac commented Oct 10, 2014

Uh oh!

jamesamcl commented Oct 10, 2014

Uh oh!

LB-- commented Oct 10, 2014

Uh oh!

cabeaulac commented Oct 10, 2014

Uh oh!

cabeaulac commented Oct 10, 2014

Uh oh!

cabeaulac commented Oct 10, 2014

Uh oh!

cabeaulac commented Oct 10, 2014

Uh oh!

jamesamcl commented Oct 11, 2014

Uh oh!

cabeaulac commented Oct 11, 2014

Uh oh!

LB-- commented Oct 11, 2014

Uh oh!

cabeaulac commented Oct 13, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants