Skip to content

Commit

Permalink
revert default changes and fix map
Browse files Browse the repository at this point in the history
  • Loading branch information
@nathanhhughes
nathanhhughes committed Sep 17, 2024
1 parent 246905d commit 39eb0f2
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 48 deletions.
35 changes: 20 additions & 15 deletions config_utilities/include/config_utilities/internal/visitor_impl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#pragma once

#include <exception>
#include <iostream>
#include <map>
#include <memory>
#include <set>
Expand Down Expand Up @@ -113,7 +112,6 @@ MetaData Visitor::subVisit(ConfigT& config,
const std::string name_space = joinNamespace(current_visitor.name_space, sub_ns);
switch (current_visitor.mode) {
case Visitor::Mode::kGet:
case Visitor::Mode::kGetDefaults:
data = getValues(config, print_warnings, name_space, field_name);
break;
case Visitor::Mode::kSet:
Expand Down Expand Up @@ -208,12 +206,16 @@ void Visitor::visitField(T& field, const std::string& field_name, const std::str
template <typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type>
void Visitor::visitField(ConfigT& config, const std::string& field_name, const std::string& name_space) {
Visitor& visitor = Visitor::instance();
if (visitor.mode == Visitor::Mode::kGetDefaults) {
return;
}

// Visit subconfig.
MetaData& data = visitor.data;
MetaData& new_data = data.sub_configs.emplace_back(Visitor::subVisit(config, false, field_name, name_space));

// Aggregate data.
if (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetDefaults) {
if (visitor.mode == Visitor::Mode::kGet) {
// When getting data add the new data also to the parent data node. This is automatically using the correct
// namespace.
mergeYamlNodes(data.data, new_data.data);
Expand All @@ -224,11 +226,15 @@ void Visitor::visitField(ConfigT& config, const std::string& field_name, const s
template <typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type>
void Visitor::visitField(std::vector<ConfigT>& config, const std::string& field_name, const std::string& /* unit */) {
Visitor& visitor = Visitor::instance();
if (visitor.mode == Visitor::Mode::kGetDefaults) {
return;
}

if (visitor.mode == Visitor::Mode::kSet) {
const auto array_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name;
const auto subnode = lookupNamespace(visitor.data.data, array_ns);
if (!subnode) {
return;
return; // don't override the field if not present
}

// When setting the values first allocate the correct amount of configs.
Expand All @@ -242,7 +248,7 @@ void Visitor::visitField(std::vector<ConfigT>& config, const std::string& field_
}
}

if (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetDefaults) {
if (visitor.mode == Visitor::Mode::kGet) {
const std::string name_space = joinNamespace(visitor.name_space, field_name);
YAML::Node array_node(YAML::NodeType::Sequence);
size_t index = 0;
Expand Down Expand Up @@ -273,11 +279,8 @@ void Visitor::visitField(std::map<K, ConfigT>& config, const std::string& field_
return;
}

OrderedMap<K, ConfigT> intermediate;
// copy current config state if doing something else other than settings the config
if (visitor.mode != Visitor::Mode::kSet) {
intermediate.insert(intermediate.begin(), config.begin(), config.end());
}
// copy current config state
OrderedMap<K, ConfigT> intermediate(config.begin(), config.end());

// make use of more general named parsing
visitField<K, ConfigT>(intermediate, field_name, unit);
Expand All @@ -300,10 +303,15 @@ void Visitor::visitField(OrderedMap<K, ConfigT>& config, const std::string& fiel
}

if (visitor.mode == Visitor::Mode::kSet) {
const auto map_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name;
const auto subnode = lookupNamespace(visitor.data.data, map_ns);
if (!subnode) {
return; // don't override the field if not present
}

// When setting the values first allocate the correct amount of configs.
config.clear();
const auto map_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name;
const auto nodes = getNodeMap(lookupNamespace(visitor.data.data, map_ns));
const auto nodes = getNodeMap(subnode);
for (auto&& [key, node] : nodes) {
auto& entry = config.emplace_back();
entry.first = key.template as<K>();
Expand Down Expand Up @@ -391,7 +399,6 @@ void Visitor::flagDefaultValues(const ConfigT& config, MetaData& data) {
size_t default_idx = 0;
for (size_t i = 0; i < data.field_infos.size(); ++i) {
FieldInfo& info = data.field_infos.at(i);
std::cout << "Looking for " << info.name << std::endl;
// note that default config may not contain the same fields as the current config
// if certain fields are conditionally enabled
for (; default_idx < default_data.field_infos.size(); ++default_idx) {
Expand All @@ -407,8 +414,6 @@ void Visitor::flagDefaultValues(const ConfigT& config, MetaData& data) {
// NOTE(lschmid): Operator YAML::Node== checks for identity, not equality. Since these are all scalars, comparing
// the formatted strings should be identical.
const auto& default_info = default_data.field_infos.at(default_idx);
std::cout << "Result for " << info.name << ": " << internal::dataToString(info.value) << " vs. "
<< internal::dataToString(default_info.value) << std::endl;
if (internal::dataToString(info.value) == internal::dataToString(default_info.value)) {
info.is_default = true;
}
Expand Down
33 changes: 0 additions & 33 deletions config_utilities/test/tests/config_arrays.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,6 @@ bool operator==(const ConfigWithDefaultArray& lhs, const ConfigWithDefaultArray&
return lhs.configs == rhs.configs;
}

struct NestedConfigWithDefaultArray {
ConfigWithDefaultArray nested;
};

void declare_config(NestedConfigWithDefaultArray& config) {
name("NestedConfigWithDefaultArray");
field(config.nested, "nested");
}

TEST(ConfigArrays, FromYamlSeq) {
const std::string yaml_seq = R"(
- s: "a"
Expand Down Expand Up @@ -448,28 +439,4 @@ TEST(ConfigArrays, ArrayWithDefaults) {
}
}

TEST(ConfigArrays, ArrayWithDefaultsDefaultMark) {
{ // make sure not specifying anything results in default
const NestedConfigWithDefaultArray config_array;
const auto result = toString(config_array);
const auto expected = R"(========================= NestedConfigWithDefaultArray =========================
nested [ConfigWithDefaultArray] (default):
configs[0] [AddString]:
s: world!
configs[1] [AddString]:
s: hello
================================================================================)";
EXPECT_EQ(result, expected);
}

{ // make sure an empty config is not marked as default
const NestedConfigWithDefaultArray config_array{{{}}};
const auto result = toString(config_array);
const auto expected = R"(========================= NestedConfigWithDefaultArray =========================
nested [ConfigWithDefaultArray]:
================================================================================)";
EXPECT_EQ(result, expected);
}
}

} // namespace config::test
28 changes: 28 additions & 0 deletions config_utilities/test/tests/config_maps.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,17 @@ void declare_config(ConfigWithNestedMaps& config) {
field(config.nested, "nested");
}

struct ConfigWithDefaultMap {
std::map<std::string, MapConfig> configs{{"name1", {"world!", 1}}, {"name2", {"hello", 3}}};
};

void declare_config(ConfigWithDefaultMap& config) {
name("ConfigWithDefaultMap");
field(config.configs, "configs");
}

bool operator==(const ConfigWithDefaultMap& lhs, const ConfigWithDefaultMap& rhs) { return lhs.configs == rhs.configs; }

TEST(ConfigMaps, FromYamlMap) {
const std::string yaml_map = R"(
x:
Expand Down Expand Up @@ -278,4 +289,21 @@ config_map[4] [MapConfig]:
EXPECT_EQ(formatted, expected);
}

TEST(ConfigMaps, MapWithDefault) {
{ // make sure not specifying anything results in default
const auto node = YAML::Load("");
auto config = fromYaml<ConfigWithDefaultMap>(node);
EXPECT_EQ(config.configs.size(), 2);
EXPECT_EQ(config, ConfigWithDefaultMap());
}

{ // specifying empty list clears default
const auto node = YAML::Load("configs: {}");
auto config = fromYaml<ConfigWithDefaultMap>(node);
EXPECT_EQ(config.configs.size(), 0);
ConfigWithDefaultMap expected{{}};
EXPECT_EQ(config, expected);
}
}

} // namespace config::test

0 comments on commit 39eb0f2

Please sign in to comment.