Skip to content

Precision #381

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

Merged
merged 3 commits into from
Oct 15, 2015
Merged

Precision #381

merged 3 commits into from
Oct 15, 2015

Conversation

bknecht
Copy link
Contributor

@bknecht bknecht commented Oct 15, 2015

My suggestion on how to fix issue #284. Sorry, it took so long. Stressful work and vacations delayed this.

StreamWriterBuilder has now a setting precision that can take values between 0 and 17. Default is always 17 as well as with deprecated classes where there is no option to limit precision.

Also added a small test to test different issues with precision and with changing the precision setting.

I hope this is solving the issue. Let me know if something is missing in your opinion.

For now use hardcoded precision '17' for now
adding precision as settings value for StreamBuilder
cdunn2001 added a commit that referenced this pull request Oct 15, 2015
@cdunn2001 cdunn2001 merged commit 772f634 into open-source-parsers:master Oct 15, 2015
@cdunn2001
Copy link
Contributor

We'll revert if somebody complains, but I can't imagine what could go wrong. Thanks!

@KarateSnowMachine
Copy link

Any idea if this is cherry-pickable to 0.y.z?

@bknecht
Copy link
Contributor Author

bknecht commented Nov 9, 2015

The change does not use any C++11 features if that's what you mean. I don't know the 0.y.z branch, but if C++11 compatibility is the only issue, the change should be safe to cherry-pick.

@cdunn2001
Copy link
Contributor

We don't have any automated tests on that branch. Could you cherry-pick and test it? Then submit a pull-request against that branch?

@bknecht
Copy link
Contributor Author

bknecht commented Nov 12, 2015

I'll allocate some time this month to test this.

@KarateSnowMachine
Copy link

I'm planning on trying to cherry-pick this sometime in the next week or two; I'll let you know how it goes.

@KarateSnowMachine
Copy link

Welp, "next week or two" certainly didn't pan out, but I've finally cherry-picked this back to 0.x.y and submitted #408

@shuhongwang-zzu
Copy link

Maybe the version of jsoncpp is inconsistent, mine is
JSONCPP_STRING valueToString(double value,
unsigned int precision,
PrecisionType precisionType) {
return valueToString(value, false, precision, precisionType);
}
I have no ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants