fix: prettier floating point numbers

[SGSSGene]

trafficstars

fixes #1289.

This PR adds a new function fp_to_string. fp_to_string internally uses dragonbox to compute the required precision to print floating point numbers. This avoids uglification of floating point numbers that happen by default via std::stringstream.

Numbers like 34.34 will be converted to '34.340000000000003' as strings. With this version they will be converted to the string '34.34'.

• [x] adjust dragonbox.h so everything is wrapped in a YAML namespace
• [x] discuss possible license adjustments for dragonbox.h
• [x] move dragonbox.h to contrib
• [x] requires OK of licensing from jk-jeon
• [x] make sure this solves the issue.

[jbeder]

@jk-jeon, given your expertise, would you be willing to review this?

[SGSSGene]

@Anton3: Could you check that this PR is actually solving your issue?

@jk-jeon: Can you check if the license in changes in a include/yaml-cpp/contrib/dragonbox.h to your liking? (I have tripled licensed them).

I belive this PR is ready to go.

[jk-jeon]

@jk-jeon: Can you check if the license in changes in a include/yaml-cpp/contrib/dragonbox.h to your liking? (I have tripled licensed them).

Looks good. Thanks!

[davidzchen]

It looks like the indentation style of the new code is inconsistent with that of the rest of the codebase (4 vs 2 spaces). @SGSSGene can you make the style consistent?

[SGSSGene]

It looks like the indentation style of the new code is inconsistent with that of the rest of the codebase (4 vs 2 spaces). @SGSSGene can you make the style consistent?

Yes, very good points.

Should I also make a fp_to_string.cpp file? Most things don't really have to be in the header file.

[SGSSGene]

It looks like the indentation style of the new code is inconsistent with that of the rest of the codebase (4 vs 2 spaces). @SGSSGene can you make the style consistent?

Yes, very good points.

Should I also make a fp_to_string.cpp file? Most things don't really have to be in the header file.

I split fp_to_string.h into src/fptostring.cpp and include/yaml-cpp/fptostring.h. This also allowed to move dragonbox.h into src/contrib folder.

[SGSSGene]

@jbeder an you help me out with the current windows-shared-build errors? (I currently don't have a windows machine at hand). I thought adding YAML_CPP_API in fptostring.h would be enough. But it seems like the symbols are not being exported into the DLL?

[jbeder]

Honestly I don't know, I don't have a Windows machine any more either :)

Maybe ask on Stack Overflow? I'd be shooting in the dark also.

[SGSSGene]

Honestly I don't know, I don't have a Windows machine any more either :)

Maybe ask on Stack Overflow? I'd be shooting in the dark also.

Found the issue: the fptostring.cpp didn't include fptostring.h, which than didnt see the "YAML_CPP_API" declaration, which then didn't export the symbols into the dll. Fixed now!

Ready to go from my side :-)

[Anton3]

@jbeder Could you give another round of review, please? Can't wait to stop uglifying my configs :)

[SGSSGene]

Hi @jbeder , any chance you can take another look?

[SGSSGene]

Hey @jbeder, a polite reminder that this is still open :-)

[davidzchen]

Sorry for the delay. Changes LGTM on my side.

@jbeder Can you please take a look at this?

[davidzchen]

@jbeder Are there any other blockers before this can be merged?

[SGSSGene]

@jbeder, any thing we can do, to motivate you looking at this PR?

[jk-jeon]

Also, has the owner of dragonbox approved this PR?

I approved the license notification. If you are talking about the implementation itself, I still think precision specification is fundamentally incompatible with the shortest roundtripping representation and don't recommend mixing them together, but if 100% correct rounding/roundtrip is not your goal than it seems okay.

[SGSSGene]

@jbeder Hey jbeder, friendly reminder, I made the adjustments you suggested to make.