Skip to content

Conversation

@amishn2008
Copy link
Contributor

Add support for scientific notation

Fixes #260 @adisidev

@amishn2008
Copy link
Contributor Author

Thank you for working on adding support for scientific notation.

src/misc/string_to_decimal_converter.cpp is an error-prone part of the software. Please add the tests for the new cases to: tests/unit/test_string_to_decimal_converter.cpp

Shifting @nihalzp comments here:
#273 (comment)

@amishn2008
Copy link
Contributor Author

This issue arose while testing this:
#283

@nihalzp
Copy link
Collaborator

nihalzp commented Jun 12, 2025

We use clang-format to format the C++ files. The style check is complaining that the file is not properly clang formatted. One way to solve it is to install clang-format, set clang-format as your default formatter, and running the auto-format command on your IDE.

Copy link
Collaborator

@adisidev adisidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amishn2008

Bumping this too. I think simply using "Format" should work as your IDE will be smart enough to detect the presence of a .clang-forrmat file and use that as the style guide for formatting. Lmk.

@nihalzp
Copy link
Collaborator

nihalzp commented Aug 4, 2025

Run mapfile -t FILES < <(git ls-files '*.[ch]pp' '*.c' '*.h')
clang-format version: Ubuntu clang-format version 18.1.3 (1ubuntu1)
Running clang-format diff...
--- include/string_to_decimal_converter.hpp	2025-06-22 00:50:12.272782406 +0000
+++ /dev/fd/63	2025-06-22 00:50:29.533764039 +0000
@@ -5,13 +5,14 @@
 #include <vector>
 
 /**
- * @brief A utility class for converting string representations of numbers to decimal format.
- * 
+ * @brief A utility class for converting string representations of numbers to
+ * decimal format.
+ *
  * This class handles various number formats including:
  * - Regular decimal numbers (e.g. "123.456", "123,456")
  * - Scientific notation (e.g. "1.23e-4", "1.23E4")
  * - Special value "NA"
- * 
+ *
  * For scientific notation:
  * - Both 'e' and 'E' are supported as exponent markers
  * - The mantissa can use either '.' or ',' as decimal separator
❌ include/string_to_decimal_converter.hpp
--- src/misc/string_to_decimal_converter.cpp	2025-06-22 00:50:12.427785355 +0000
+++ /dev/fd/63	2025-06-22 00:50:30.488765944 +0000
@@ -7,7 +7,8 @@
 
 bool StringToDecimalConverter::is_valid_char(char ch)
 {
-  return (std::isdigit(ch)) || ch == point_ || ch == comma_ || ch == minus_ || ch == 'e' || ch == 'E';
+  return (std::isdigit(ch)) || ch == point_ || ch == comma_ || ch == minus_ ||
+         ch == 'e' || ch == 'E';
 }
 
 std::string StringToDecimalConverter::remove_char(std::string str, char ch)
❌ src/misc/string_to_decimal_converter.cpp
Error: clang-format issues detected in the following files:
include/string_to_decimal_converter.hpp
src/misc/string_to_decimal_converter.cpp
Run 'clang-format -i' to fix.

The clang-format is failing. Just a simple auto format command on your editor for the file include/string_to_decimal_converter.hpp will do the trick.

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.

Allow scientific notation for CSV files

3 participants