Skip to content

Commit

Permalink
fix float formatting test errors
Browse files Browse the repository at this point in the history
  • Loading branch information
nathanhhughes committed Dec 3, 2023
1 parent 9441b21 commit 2e85dec
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ std::string joinNamespace(const std::string& namespace_1,
* @param data The data to be formatted.
* @returns The formatted string.
*/
std::string dataToString(const YAML::Node& data);
std::string dataToString(const YAML::Node& data, bool reformat_float = false);

/**
* @brief Find all occurences of a substring in a string.
Expand Down
3 changes: 3 additions & 0 deletions config_utilities/include/config_utilities/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ struct Settings {
// If true, store all validated configs for global printing.
bool store_valid_configs = true;

// If true, attempts to print floats and float-like fields with default stream precision
bool reformat_floats = true;

/* Factory settings */
// The factory will look for this param to deduce the type of the object to be created.
std::string factory_type_param_name = "type";
Expand Down
3 changes: 2 additions & 1 deletion config_utilities/src/asl_formatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,10 @@ std::string AslFormatter::formatField(const FieldInfo& info, size_t indent) cons
std::string result;
const size_t print_width = Settings::instance().print_width;
const size_t global_indent = Settings::instance().print_indent;
const auto reformat_floats = Settings::instance().reformat_floats;

// field is the stringified value, The header is the field name.
std::string field = dataToString(info.value);
std::string field = dataToString(info.value, reformat_floats);
if (info.is_default && Settings::instance().indicate_default_values) {
field += " (default)";
}
Expand Down
39 changes: 32 additions & 7 deletions config_utilities/src/string_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "config_utilities/internal/string_utils.h"

#include <algorithm>
#include <regex>
#include <sstream>

namespace config::internal {
Expand Down Expand Up @@ -88,18 +89,41 @@ std::string joinNamespace(const std::string& namespace_1,
return joinNamespace(ns_1, delimiter);
}

std::string dataToString(const YAML::Node& data) {
std::string scalarToString(const YAML::Node& data, bool reformat_float) {
std::stringstream orig;
orig << data;
if (!reformat_float) {
return orig.str();
}

const std::regex float_detector("[+-]?[0-9]*[.][0-9]+");
if (!std::regex_search(orig.str(), float_detector)) {
return orig.str(); // no reason to reformat if no decimal points
}

double value;
try {
value = data.as<double>();
} catch (const YAML::InvalidNode&) {
return orig.str(); // value is some sort of string that can't be parsed as a float
}

// this should have default ostream precision for formatting float
std::stringstream ss;
ss << value;
return ss.str();
}

std::string dataToString(const YAML::Node& data, bool reformat_float) {
switch (data.Type()) {
case YAML::NodeType::Scalar: {
// NOTE(lschmid): All YAML scalars should implement the ostream<< operator.
std::stringstream ss;
ss << data;
return ss.str();
// scalars require special handling for float precision
return scalarToString(data, reformat_float);
}
case YAML::NodeType::Sequence: {
std::string result = "[";
for (size_t i = 0; i < data.size(); ++i) {
result += dataToString(data[i]);
result += dataToString(data[i], reformat_float);
if (i < data.size() - 1) {
result += ", ";
}
Expand All @@ -112,7 +136,8 @@ std::string dataToString(const YAML::Node& data) {
bool has_data = false;
for (const auto& kv_pair : data) {
has_data = true;
result += dataToString(kv_pair.first) + ": " + dataToString(kv_pair.second) + ", ";
result +=
dataToString(kv_pair.first, reformat_float) + ": " + dataToString(kv_pair.second, reformat_float) + ", ";
}
if (has_data) {
result = result.substr(0, result.length() - 2);
Expand Down
4 changes: 2 additions & 2 deletions config_utilities/test/src/default_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ YAML::Node DefaultConfig::modifiedValues() {
YAML::Node data;
data["i"] = 2;
data["f"] = -1.f;
data["d"] = 3.1415926;
data["d"] = 3.14159; // intentionally avoid precision issues
data["b"] = false;
data["u8"] = 255;
data["s"] = "a different test string";
Expand Down Expand Up @@ -158,7 +158,7 @@ void DefaultConfig::expectDefaultValues() {
void DefaultConfig::expectModifiedValues() {
EXPECT_EQ(i, 2);
EXPECT_EQ(f, -1.f);
EXPECT_EQ(d, 3.1415926);
EXPECT_EQ(d, 3.14159);
EXPECT_EQ(b, false);
EXPECT_EQ(u8, 255);
EXPECT_EQ(s, "a different test string");
Expand Down
29 changes: 17 additions & 12 deletions config_utilities/test/tests/asl_formatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,22 +91,24 @@ void declare_config(ConfigUsingArrays& config) {

TEST(AslFormatter, DataToString) {
YAML::Node data = internal::Visitor::getValues(TestConfig()).data;
EXPECT_EQ(internal::dataToString(data["i"]), "1");
EXPECT_EQ(internal::dataToString(data["f"]), "2.1");
EXPECT_EQ(internal::dataToString(data["d"]), "3.2");
EXPECT_EQ(internal::dataToString(data["b"]), "true");
EXPECT_EQ(internal::dataToString(data["u8"]), "4");
EXPECT_EQ(internal::dataToString(data["s"]), "test string");
EXPECT_EQ(internal::dataToString(data["vec"]), "[1, 2, 3]");
EXPECT_EQ(internal::dataToString(data["map"]), "{a: 1, b: 2, c: 3}");
EXPECT_EQ(internal::dataToString(data["set"]), "[1.1, 2.2, 3.3]");
EXPECT_EQ(internal::dataToString(data["mat"]), "[[1, 0, 0], [0, 1, 0], [0, 0, 1]]");
// note: float reformatting should have no effect on other fields (and fixes full precision formatting for internal
// yaml representation)
EXPECT_EQ(internal::dataToString(data["i"], true), "1");
EXPECT_EQ(internal::dataToString(data["f"], true), "2.1");
EXPECT_EQ(internal::dataToString(data["d"], true), "3.2");
EXPECT_EQ(internal::dataToString(data["b"], true), "true");
EXPECT_EQ(internal::dataToString(data["u8"], true), "4");
EXPECT_EQ(internal::dataToString(data["s"], true), "test string");
EXPECT_EQ(internal::dataToString(data["vec"], true), "[1, 2, 3]");
EXPECT_EQ(internal::dataToString(data["map"], true), "{a: 1, b: 2, c: 3}");
EXPECT_EQ(internal::dataToString(data["set"], true), "[1.1, 2.2, 3.3]");
EXPECT_EQ(internal::dataToString(data["mat"], true), "[[1, 0, 0], [0, 1, 0], [0, 0, 1]]");
YAML::Node nested_set;
nested_set["a"]["x"] = 1;
nested_set["a"]["y"] = 2;
nested_set["b"]["x"] = 3;
nested_set["b"]["y"] = 4;
EXPECT_EQ(internal::dataToString(nested_set), "{a: {x: 1, y: 2}, b: {x: 3, y: 4}}");
EXPECT_EQ(internal::dataToString(nested_set, true), "{a: {x: 1, y: 2}, b: {x: 3, y: 4}}");
}

TEST(AslFormatter, FormatErrors) {
Expand Down Expand Up @@ -222,6 +224,7 @@ TEST(AslFormatter, FormatConfig) {
Settings().indicate_default_values = false;
Settings().indicate_units = false;
Settings().inline_subconfig_field_names = true;
Settings().reformat_floats = true;
std::string formatted = internal::Formatter::formatConfig(data);
std::string expected =
R"""(================================= Test Config ==================================
Expand Down Expand Up @@ -339,6 +342,7 @@ TEST(AslFormatter, FormatUnits) {
Settings().indicate_units = true;
Settings().inline_subconfig_field_names = true;
Settings().print_width = 80; // force print width to be consistent for tests
Settings().print_indent = 20;

internal::MetaData data = internal::Visitor::getValues(TestConfig());
const std::string formatted = internal::Formatter::formatConfig(data);
Expand Down Expand Up @@ -381,6 +385,7 @@ TEST(AslFormatter, FormatDefaultValues) {
Settings().indicate_default_values = true;
Settings().indicate_units = false;
Settings().inline_subconfig_field_names = true;
Settings().print_indent = 20;

const internal::MetaData default_data = internal::Visitor::getValues(TestConfig());
std::string formatted = internal::Formatter::formatConfig(default_data);
Expand Down Expand Up @@ -423,7 +428,7 @@ sub_sub_config [SubSubConfig] (default):
expected = R"""(================================= Test Config ==================================
i: 2
f: -1
d: 3.1415926
d: 3.14159
b: false
u8: 255
s: a different test string
Expand Down
2 changes: 2 additions & 0 deletions config_utilities/test/tests/config_arrays.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "config_utilities/formatting/asl.h"
#include "config_utilities/parsing/yaml.h"
#include "config_utilities/printing.h"
#include "config_utilities/settings.h"
#include "config_utilities/test/utils.h"
#include "config_utilities/validation.h"
#include "config_utilities/virtual_config.h"
Expand Down Expand Up @@ -340,6 +341,7 @@ TEST(ConfigArrays, PrintArrayConfigs) {
configs.emplace_back("a", 1.0f);
configs.emplace_back("b", 2.0f);
configs.emplace_back("c", 3.0f);
Settings().print_indent = 20;

internal::Formatter::setFormatter(std::make_unique<internal::AslFormatter>());

Expand Down
2 changes: 1 addition & 1 deletion config_utilities/test/tests/yaml_parsing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ TEST(YamlParsing, parsefromYaml) {
EXPECT_EQ(config.f, -1.f);

internal::YamlParser::fromYaml(data, "d", config.d, "", error);
EXPECT_EQ(config.d, 3.1415926);
EXPECT_EQ(config.d, 3.14159);

internal::YamlParser::fromYaml(data, "b", config.b, "", error);
EXPECT_EQ(config.b, false);
Expand Down

0 comments on commit 2e85dec

Please sign in to comment.