Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: JParameterManager comma escaping #401

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Feature: JParameterManager comma escaping #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f6a2ae9
Add test for escaping commas. Warning: Does not pass ctests
rlatosky Jan 29, 2025
a17e480
Fixed issue with tests. Modified algorithm to recognize escape char
rlatosky Jan 31, 2025
7dd5986
Add additional tests for parsing vectors and arrays with escape keys
rlatosky Feb 3, 2025
fb15df6
Merge branch 'JeffersonLab:master' into testing
rlatosky Feb 3, 2025
a8c5f35
Revert changing main parse function. Add templates for strings of arr…
rlatosky Feb 4, 2025
18a1d56
Fix templating error/warning with clang
rlatosky Feb 5, 2025
577d1db
Merge branch 'JeffersonLab:master' into testing
rlatosky Feb 5, 2025
e5a1a6d
Reconfigure tests and apply recommendations from PR
rlatosky Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion src/libraries/JANA/Services/JParameterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ class JParameterManager : public JService {
template<typename T, size_t arrSize>
static inline void Parse(const std::string& value, std::array<T,arrSize>& out);

template<size_t arrSize>
static inline void Parse(const std::string& value, std::array<std::string,arrSize>& out);


/*
Template specialization done for double and float numbers in order to match the
precision of the number with the string
Expand Down Expand Up @@ -374,7 +378,33 @@ inline void JParameterManager::Parse(const std::string& value,std::array<T,N> &v
T t;
Parse(s, t);
val[indx++]= t;
}
}
}

// @brief Template to parse a string and return in an array of strings
template<size_t N>
inline void JParameterManager::Parse(const std::string& value,std::array<std::string,N> &val) {
std::string s;
std::stringstream ss(value);
std::string temp = ""; // creating a temp var allows us to store the string s where the escape character was used
int indx = 0;
while (getline(ss, s, ',')) {
std::string t;
if (s.back() == '\\') {
s.pop_back();
temp += s + ',';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When find yourself concatenating a string inside a loop, you should probably be using a string builder instead, because it will have much better time complexity. In C++ you would use std::ostringstream

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Never mind, this might not be a problem in std::string. It certainly was a problem in other contexts such as java's String class

continue;
}
if (!temp.empty()) {
Parse(temp + s, t);
val[indx++]= t;
temp.clear();
}
else {
Parse(s, t);
val[indx++]= t;
}
}
}

/// @brief Specialization for std::vector<std::string>
Expand All @@ -390,6 +420,32 @@ inline void JParameterManager::Parse(const std::string& value, std::vector<T> &v
}
}

/// @brief Specialization for std::vector<std::string> with escape commas
template<>
inline void JParameterManager::Parse(const std::string& value, std::vector<std::string> &val) {
std::stringstream ss(value);
std::string s;
val.clear(); // clearing the input vector to ensure no dulication which can be caused due to val.push_back(t);
std::string temp = ""; // creating a temp var allows us to store the string s where the escape character was used
while (getline(ss, s, ',')) {
std::string t;
if (s.back() == '\\') {
s.pop_back();
temp += s + ',';
continue;
}
if (!temp.empty()) {
Parse(temp + s, t);
val.push_back(t);
temp.clear();
}
else {
Parse(s, t);
val.push_back(t);
}
}
}

/// @brief Specialization for JLogger::Level enum
template <>
inline void JParameterManager::Parse(const std::string& in, JLogger::Level& out) {
Expand Down
55 changes: 55 additions & 0 deletions src/programs/unit_tests/Services/JParameterManagerTests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@


#include <JANA/Services/JParameterManager.h>
#include <vector>
#include "catch.hpp"

TEST_CASE("JParameterManager::SetDefaultParameter") {
Expand Down Expand Up @@ -237,6 +238,33 @@ TEST_CASE("JParameterManager_VectorParams") {
auto param = jpm.GetParameter("test", outputs);
REQUIRE(param->GetValue() == "22,49.2,42");
}
SECTION("Reading a vector of functions with commas") {
// As of Mon Jan 27, JParameterManager does not allow an escape key to prevent splitting on the next comma)
jpm.SetParameter("test", "phi-fmod(phi\\,5),theta-fmod(theta\\,10),omega-fmod(omega\\,15)"); // Issue #380 (Feature request)
std::vector<std::string> vals;
jpm.GetParameter<std::vector<std::string>>("test", vals);

REQUIRE(vals[0] == "phi-fmod(phi,5)");
REQUIRE(vals[1] == "theta-fmod(theta,10)");
REQUIRE(vals[2] == "omega-fmod(omega,15)");
}
SECTION("Writing a vector of functions with commas") {
std::vector<std::string> inputs;
inputs.emplace_back("phi-fmod(phi\\,5)");
inputs.emplace_back("theta-fmod(theta\\,10)");
inputs.emplace_back("omega-fmod(omega\\,15)");
std::vector<std::string> temp1 = inputs;

jpm.SetDefaultParameter("test", inputs);
std::vector<std::string> outputs;
auto param = jpm.GetParameter("test", outputs);
REQUIRE(param->GetValue() == "phi-fmod(phi\\,5),theta-fmod(theta\\,10),omega-fmod(omega\\,15)");
REQUIRE(inputs.size()==3);

std::vector<std::string> temp2;
jpm.Parse(jpm.Stringify(temp1), temp2);
REQUIRE(temp2 == outputs);
}
}

TEST_CASE("JParameterManager::RegisterParameter") {
Expand Down Expand Up @@ -316,6 +344,33 @@ TEST_CASE("JParameterManager_ArrayParams") {
auto param = jpm.GetParameter("test", outputs);
REQUIRE(param->GetValue() == "22,49.2,42");
}
SECTION("Reading a array of functions with commas") {
// As of Mon Jan 27, JParameterManager does not allow an escape key to prevent splitting on the next comma)
jpm.SetParameter("test", "theta-fmod(phi-fmod(phi\\,5)\\,7),theta-fmod(theta\\,10),omega-fmod(omega\\,15)"); // Issue #380 (Feature request)
std::array<std::string, 3> vals;
jpm.GetParameter("test", vals);

REQUIRE(vals[0] == "theta-fmod(phi-fmod(phi,5),7)");
REQUIRE(vals[1] == "theta-fmod(theta,10)");
REQUIRE(vals[2] == "omega-fmod(omega,15)");
}
SECTION("Writing a array of functions with commas") {
std::array<std::string, 3> inputs = {
"theta-fmod(phi-fmod(phi\\,5)\\,7)",
"theta-fmod(theta\\,10)",
"omega-fmod(omega\\,15)"
};
std::array<std::string,3> temp1 = inputs;

jpm.SetDefaultParameter("test", inputs);
std::array<std::string,3> outputs;
auto param = jpm.GetParameter("test", outputs);
REQUIRE(param->GetValue() == "theta-fmod(phi-fmod(phi\\,5)\\,7),theta-fmod(theta\\,10),omega-fmod(omega\\,15)");

std::array<std::string,3> temp2;
jpm.Parse(jpm.Stringify(temp1), temp2);
REQUIRE(temp2 == outputs);
}
}

TEST_CASE("JParameterManagerFloatingPointRoundTrip") {
Expand Down