search

vincentlaucsb / csv-parser

A high-performance, fully-featured CSV parser and serializer for modern C++.
MIT License
901 stars 150 forks source link

Fix conversion when integral part is 0 #216

Closed ahoarau closed 6 months ago

sddale commented 11 months ago

I ran into the same problem and found the same solution. Please merge so others don't generate incorrect csvs, knowingly or otherwise.

ahoarau commented 7 months ago

@vincentlaucsb ?

vincentlaucsb commented 6 months ago

@ahoarau I'm not sure what exactly is the issue here, or what your proposed modification does. Do you have any example cases where this function fails?

If the integral part is 0, this branch of the code shouldn't even get executed.

Furthermore, ln(x)/ln(10) = log10(x). std::log() is the natural logarithm, so again we're not really changing the math here.

ahoarau commented 6 months ago

Apologies for not putting a description. Basically this does not pass on master (csv_test)

  SECTION("Tuple") {
    writer << std::make_tuple(500.0, -500.0, 1000.0, 0.0, 0.0);
    correct << "500.0,-500.0,1000.0,0.0,0.0";
  }
csv-parser\tests\test_write_csv.cpp(71): FAILED:
  REQUIRE( output.str() == correct.str() )
with expansion:
  "500.0,-500.0,000.0,0.0,0.0
  "
  ==
  "500.0,-500.0,1000.0,0.0,0.0
  "

It passes on my branch, but is incompatible with https://github.com/vincentlaucsb/csv-parser/commit/a6ce8b2d4fb37cc9b7655d636cb0c3b46628818b

Any help would be appreciated

vincentlaucsb commented 6 months ago

I believe the latest release should solve this. This appears to be a duplicate of #188.

I have decided to re-write the digit counting functions without the standard library logarithm functions. Per the discussion in a StackOverflow article linked in the latest release notes, they are not reliable for calculating digits.

Please feel free to open another issue if this problem persists.