Skip to content

Commit caf4d15

Browse files
GlassCopilot
Glass
and
Copilot
authored
Updates to OptionalBool (#39241) (#39269)
Sister PR to #39241 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent ad419c8 commit caf4d15

File tree

9 files changed

+129
-22
lines changed

9 files changed

+129
-22
lines changed

‎Framework/Algorithms/src/Divide.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,10 @@ void Divide::setOutputUnits(const API::MatrixWorkspace_const_sptr lhs, const API
103103
}
104104

105105
// override `isDistribution` if user provided
106-
if (this->getPropertyValue("IsDistribution") == OptionalBool::StrTrue)
106+
OptionalBool isDistribution = this->getProperty("IsDistribution");
107+
if (isDistribution == OptionalBool::Value::True)
107108
out->setDistribution(true);
108-
else if (this->getPropertyValue("IsDistribution") == OptionalBool::StrFalse)
109+
else if (isDistribution == OptionalBool::Value::False)
109110
out->setDistribution(false);
110111
}
111112

‎Framework/Algorithms/src/Stitch1DMany.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ void Stitch1DMany::init() {
4545
"End overlaps for stitched workspaces "
4646
"(number of input workspaces minus one).");
4747

48+
// This property is allowed to be Unset, as it no longer has an effect
4849
declareProperty(
4950
std::make_unique<PropertyWithValue<OptionalBool>>("ScaleRHSWorkspace", OptionalBool::Unset, Direction::Input),
5051
"Scaling either with respect to first (first hand side, LHS) "

‎Framework/Algorithms/test/MultiplyDivideTest.in.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,29 @@ class @MULTIPLYDIVIDETEST_CLASS@ : public CxxTest::TestSuite
7676
TS_ASSERT_THROWS( alg->setPropertyValue("LHSWorkspace","test_in21"), const std::invalid_argument &);
7777
TS_ASSERT_THROWS( alg->setPropertyValue("RHSWorkspace","test_in22"), const std::invalid_argument &);
7878
TS_ASSERT_THROWS_NOTHING( alg->setPropertyValue("OutputWorkspace","test_out2") );
79+
80+
if (DO_DIVIDE)
81+
{
82+
// Test OptionalBool
83+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool(OptionalBool::True)));
84+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool(OptionalBool::False)));
85+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool(true)));
86+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool(false)));
87+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool("True")));
88+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool("true")));
89+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool("False")));
90+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool("false")));
91+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool(0)));
92+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", OptionalBool(1)));
93+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", "True"));
94+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", "False"));
95+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", "true"));
96+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", "false"));
97+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", "0"));
98+
TS_ASSERT_THROWS_NOTHING( alg->setProperty("IsDistribution", "1"));
99+
TS_ASSERT_THROWS( alg->setProperty("IsDistribution", "jimmy"), const std::invalid_argument &);
100+
}
101+
79102
delete alg;
80103
}
81104

‎Framework/Kernel/inc/MantidKernel/OptionalBool.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ namespace Kernel {
2424
*/
2525
class MANTID_KERNEL_DLL OptionalBool {
2626
public:
27-
enum Value { Unset, True, False };
28-
static std::map<std::string, Value> strToEmumMap();
27+
enum Value : unsigned char { False, True, Unset };
28+
static std::map<std::string, Value> strToEnumMap();
2929
static std::map<Value, std::string> enumToStrMap();
3030
const static std::string StrUnset;
3131
const static std::string StrFalse;
@@ -35,10 +35,19 @@ class MANTID_KERNEL_DLL OptionalBool {
3535
OptionalBool();
3636
OptionalBool(bool arg);
3737
OptionalBool(Value arg);
38-
virtual ~OptionalBool() = default;
38+
OptionalBool(std::string arg);
39+
OptionalBool(char const *arg);
40+
OptionalBool(const int arg);
41+
OptionalBool(const OptionalBool &other) = default;
42+
OptionalBool &operator=(const OptionalBool &other) = default;
43+
OptionalBool &operator=(std::string const &arg);
44+
OptionalBool &operator=(char const *arg);
45+
OptionalBool &operator=(const int arg);
3946
bool operator==(const OptionalBool &other) const;
4047
bool operator!=(const OptionalBool &other) const;
4148
Value getValue() const;
49+
Value Validate(const std::string &arg);
50+
virtual ~OptionalBool() = default;
4251

4352
private:
4453
friend MANTID_KERNEL_DLL std::ostream &operator<<(std::ostream &os, OptionalBool const &object);

‎Framework/Kernel/inc/MantidKernel/PropertyHelper.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,18 @@ template <typename T> void toValue(const std::string &strvalue, T &value) { valu
209209

210210
template <typename T> void toValue(const std::string &, std::shared_ptr<T> &) { throw boost::bad_lexical_cast(); }
211211

212+
/** Helper functions for setting the value of an OptionalBool property */
213+
template <> [[maybe_unused]] void toValue(const std::string &strValue, OptionalBool &value) {
214+
const auto normalizedStr = Mantid::Kernel::Strings::toLower(strValue);
215+
if (normalizedStr == "0" || normalizedStr == "false") {
216+
value = OptionalBool::False;
217+
} else if (normalizedStr == "1" || normalizedStr == "true") {
218+
value = OptionalBool::True;
219+
} else {
220+
value = strValue;
221+
}
222+
}
223+
212224
namespace detail {
213225
// vector<int> specializations
214226
template <typename T> void toValue(const std::string &strvalue, std::vector<T> &value, std::true_type) {

‎Framework/Kernel/src/OptionalBool.cpp‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,56 @@
88

99
#include <json/value.h>
1010

11+
#include <algorithm>
1112
#include <ostream>
1213
#include <utility>
1314

1415
namespace Mantid::Kernel {
1516

16-
const std::string OptionalBool::StrUnset = "Unset";
1717
const std::string OptionalBool::StrFalse = "False";
1818
const std::string OptionalBool::StrTrue = "True";
19+
const std::string OptionalBool::StrUnset = "Unset";
1920

20-
OptionalBool::OptionalBool() : m_arg(Unset) {}
21-
22-
OptionalBool::OptionalBool(bool arg) { m_arg = arg ? True : False; }
21+
OptionalBool::Value OptionalBool::Validate(const std::string &arg) {
22+
auto l_toLower = [](char c) -> char { return (c >= 'A' && c <= 'Z' ? c + 0x20 : c); };
23+
std::string argLower = arg;
24+
std::transform(arg.begin(), arg.end(), argLower.begin(), l_toLower);
25+
if (argLower == "false") {
26+
return False;
27+
} else if (argLower == "true") {
28+
return True;
29+
} else if (argLower == "unset") {
30+
return Unset;
31+
} else {
32+
throw std::invalid_argument("Invalid value for OptionalBool: " + arg);
33+
}
34+
}
2335

24-
OptionalBool::OptionalBool(Value arg) : m_arg(arg) {}
36+
OptionalBool::OptionalBool() : m_arg(Unset) {}
37+
OptionalBool::OptionalBool(bool arg) : m_arg(OptionalBool::Value(arg)) {}
38+
OptionalBool::OptionalBool(OptionalBool::Value arg) : m_arg(arg) {}
39+
OptionalBool::OptionalBool(std::string arg) : m_arg(Validate(arg)) {}
40+
OptionalBool::OptionalBool(char const *arg) : m_arg(Validate(std::string(arg))) {}
41+
OptionalBool::OptionalBool(const int arg)
42+
: m_arg(arg == 0 ? OptionalBool::False
43+
: arg == 1 ? OptionalBool::True
44+
: throw std::invalid_argument("Invalid value for OptionalBool: " + std::to_string(arg) +
45+
"\nAccepted values are 0 or 1")) {}
46+
OptionalBool &OptionalBool::operator=(std::string const &arg) {
47+
m_arg = OptionalBool::Validate(arg);
48+
return *this;
49+
}
50+
OptionalBool &OptionalBool::operator=(char const *arg) {
51+
m_arg = OptionalBool::Validate(std::string(arg));
52+
return *this;
53+
}
54+
OptionalBool &OptionalBool::operator=(const int arg) {
55+
m_arg = arg == 0 ? OptionalBool::False
56+
: arg == 1 ? OptionalBool::True
57+
: throw std::invalid_argument("Invalid value for OptionalBool: " + std::to_string(arg) +
58+
"\nAccepted values are 0 or 1");
59+
return *this;
60+
}
2561

2662
bool OptionalBool::operator==(const OptionalBool &other) const { return m_arg == other.getValue(); }
2763

@@ -37,17 +73,17 @@ std::ostream &operator<<(std::ostream &os, OptionalBool const &object) {
3773
std::istream &operator>>(std::istream &istream, OptionalBool &object) {
3874
std::string result;
3975
istream >> result;
40-
object.m_arg = OptionalBool::strToEmumMap()[result];
76+
object.m_arg = OptionalBool::strToEnumMap()[result];
4177
return istream;
4278
}
4379

44-
std::map<std::string, OptionalBool::Value> OptionalBool::strToEmumMap() {
80+
std::map<std::string, OptionalBool::Value> OptionalBool::strToEnumMap() {
4581
return {{StrUnset, OptionalBool::Unset}, {StrFalse, OptionalBool::False}, {StrTrue, OptionalBool::True}};
4682
}
4783

4884
std::map<OptionalBool::Value, std::string> OptionalBool::enumToStrMap() {
49-
std::map<Value, std::string> map;
50-
auto opposite = strToEmumMap();
85+
std::map<OptionalBool::Value, std::string> map;
86+
auto opposite = strToEnumMap();
5187
for (auto &oppositePair : opposite) {
5288
map.emplace(oppositePair.second, oppositePair.first);
5389
}

‎Framework/Kernel/test/OptionalBoolTest.h‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,50 @@ class OptionalBoolTest : public CxxTest::TestSuite {
2323
static OptionalBoolTest *createSuite() { return new OptionalBoolTest(); }
2424
static void destroySuite(OptionalBoolTest *suite) { delete suite; }
2525

26-
void test_construction_by_bool() {
26+
void test_defaults_to_unset() {
27+
OptionalBool arg;
28+
TS_ASSERT_EQUALS(OptionalBool::Unset, arg.getValue());
29+
}
2730

31+
void test_construction_by_bool() {
2832
OptionalBool arg1(true);
2933
TS_ASSERT_EQUALS(OptionalBool::True, arg1.getValue());
3034

3135
OptionalBool arg2(false);
3236
TS_ASSERT_EQUALS(OptionalBool::False, arg2.getValue());
3337
}
3438

35-
void test_defaults_to_unset() {
36-
OptionalBool arg;
37-
TS_ASSERT_EQUALS(OptionalBool::Unset, arg.getValue());
38-
}
39-
4039
void test_construction_by_value() {
4140
auto value = OptionalBool::True;
4241
OptionalBool arg(value);
4342
TS_ASSERT_EQUALS(value, arg.getValue());
4443
}
4544

45+
void test_construction_by_string() {
46+
OptionalBool arg1("True");
47+
TS_ASSERT_EQUALS(OptionalBool::True, arg1.getValue());
48+
49+
OptionalBool arg2("False");
50+
TS_ASSERT_EQUALS(OptionalBool::False, arg2.getValue());
51+
52+
OptionalBool arg3("Unset");
53+
TS_ASSERT_EQUALS(OptionalBool::Unset, arg3.getValue());
54+
55+
// Test case-insensitive
56+
OptionalBool arg4("tRuE");
57+
TS_ASSERT_EQUALS(OptionalBool::True, arg4.getValue());
58+
}
59+
60+
void test_construction_by_int() {
61+
OptionalBool arg2(0);
62+
TS_ASSERT_EQUALS(OptionalBool::False, arg2.getValue());
63+
64+
OptionalBool arg1(1);
65+
TS_ASSERT_EQUALS(OptionalBool::True, arg1.getValue());
66+
67+
TS_ASSERT_THROWS(OptionalBool arg3(2), const std::invalid_argument &);
68+
}
69+
4670
void test_comparison_overload() {
4771
OptionalBool a(OptionalBool::True);
4872
OptionalBool b(OptionalBool::True);
@@ -114,7 +138,7 @@ class OptionalBoolTest : public CxxTest::TestSuite {
114138
}
115139

116140
void test_str_map() {
117-
auto map = OptionalBool::strToEmumMap();
141+
auto map = OptionalBool::strToEnumMap();
118142
TS_ASSERT_EQUALS(3, map.size());
119143
TS_ASSERT_EQUALS(map[OptionalBool::StrUnset], OptionalBool::Unset);
120144
TS_ASSERT_EQUALS(map[OptionalBool::StrFalse], OptionalBool::False);

‎Framework/Kernel/test/PropertyWithValueTest.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ class PropertyWithValueTest : public CxxTest::TestSuite {
647647
PropertyWithValue<OptionalBool> property("myproperty", OptionalBool::Unset, Direction::Input);
648648

649649
auto values = property.allowedValues();
650-
auto possibilities = OptionalBool::strToEmumMap();
650+
auto possibilities = OptionalBool::strToEnumMap();
651651
TSM_ASSERT_EQUALS("3 states allowed", possibilities.size(), values.size());
652652
for (auto &value : values) {
653653
TSM_ASSERT("value not a known state", possibilities.find(value) != possibilities.end());

‎docs/source/release/v6.13.0/Framework/Algorithms/Bugfixes/39241.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Add `MandatoryValidator<OptionalBool>` to property declaration in Divide algorithm

0 commit comments

Comments
 (0)