From 9239e5ee07e3ca87a9731364d29555efa16fcd63 Mon Sep 17 00:00:00 2001
From: Nick Draper <nick.draper@stfc.ac.uk>
Date: Tue, 4 Jul 2017 15:55:28 +0100
Subject: [PATCH] Do not alter property.value, just add valuePrettyPrint
and make use in history and logging
re #15164
---
.../API/inc/MantidAPI/AlgorithmHistory.h | 2 +-
Framework/API/src/Algorithm.cpp | 10 ++-
Framework/API/src/AlgorithmHistory.cpp | 8 +-
Framework/Kernel/inc/MantidKernel/Property.h | 5 +-
.../Kernel/inc/MantidKernel/PropertyHelper.h | 77 ++++++++++++++-----
.../Kernel/inc/MantidKernel/PropertyHistory.h | 2 +-
.../inc/MantidKernel/PropertyWithValue.h | 2 +
.../inc/MantidKernel/PropertyWithValue.tcc | 8 ++
Framework/Kernel/inc/MantidKernel/Strings.h | 3 +
Framework/Kernel/src/Property.cpp | 13 ++++
Framework/Kernel/src/PropertyHistory.cpp | 23 +++---
Framework/Kernel/src/Strings.cpp | 25 ++++++
Framework/Kernel/test/ArrayPropertyTest.h | 11 +--
Framework/Kernel/test/PropertyHistoryTest.h | 15 ++++
Framework/Kernel/test/StringsTest.h | 63 +++++++++++++++
15 files changed, 227 insertions(+), 40 deletions(-)
diff --git a/Framework/API/inc/MantidAPI/AlgorithmHistory.h b/Framework/API/inc/MantidAPI/AlgorithmHistory.h
index 5ca1739a633..1466bbad3d0 100644
--- a/Framework/API/inc/MantidAPI/AlgorithmHistory.h
+++ b/Framework/API/inc/MantidAPI/AlgorithmHistory.h
@@ -121,7 +121,7 @@ public:
/// Retrieve the number of child algorithms
size_t childHistorySize() const;
/// print contents of object
- void printSelf(std::ostream &, const int indent = 0) const;
+ void printSelf(std::ostream &, const int indent = 0, const size_t maxPropertyLength = 0) const;
/// Less than operator
inline bool operator<(const AlgorithmHistory &other) const {
return (execCount() < other.execCount());
diff --git a/Framework/API/src/Algorithm.cpp b/Framework/API/src/Algorithm.cpp
index 92fe324c22e..2341774faea 100644
--- a/Framework/API/src/Algorithm.cpp
+++ b/Framework/API/src/Algorithm.cpp
@@ -1059,8 +1059,14 @@ void Algorithm::logAlgorithmInfo() const {
logger.notice() << '\n';
// Make use of the AlgorithmHistory class, which holds all the info we want
// here
- AlgorithmHistory AH(this);
- logger.information() << AH;
+ AlgorithmHistory algHistory(this);
+ size_t maxPropertyLength = 40;
+ if (logger.is(Logger::Priority::PRIO_DEBUG))
+ {
+ //include the full property value when logging in debug
+ maxPropertyLength = 0;
+ }
+ algHistory.printSelf(logger.information(), 0, maxPropertyLength);
}
}
diff --git a/Framework/API/src/AlgorithmHistory.cpp b/Framework/API/src/AlgorithmHistory.cpp
index cc9c2475ba6..1d4d56d51a5 100644
--- a/Framework/API/src/AlgorithmHistory.cpp
+++ b/Framework/API/src/AlgorithmHistory.cpp
@@ -201,11 +201,13 @@ AlgorithmHistory::getChildAlgorithm(const size_t index) const {
}
/** Prints a text representation of itself
- * @param os :: The ouput stream to write to
+ * @param os :: The output stream to write to
* @param indent :: an indentation value to make pretty printing of object and
* sub-objects
+ * @param maxPropertyLength :: the max length for any property value string (0 = full length)
*/
-void AlgorithmHistory::printSelf(std::ostream &os, const int indent) const {
+void AlgorithmHistory::printSelf(std::ostream &os, const int indent,
+ const size_t maxPropertyLength) const {
os << std::string(indent, ' ') << "Algorithm: " << m_name;
os << std::string(indent, ' ') << " v" << m_version << '\n';
@@ -217,7 +219,7 @@ void AlgorithmHistory::printSelf(std::ostream &os, const int indent) const {
os << std::string(indent, ' ') << "Parameters:\n";
for (const auto &property : m_properties) {
- property->printSelf(os, indent + 2);
+ property->printSelf(os, indent + 2, maxPropertyLength);
}
}
diff --git a/Framework/Kernel/inc/MantidKernel/Property.h b/Framework/Kernel/inc/MantidKernel/Property.h
index 5179ef8fe0b..f6c1eaef2c4 100644
--- a/Framework/Kernel/inc/MantidKernel/Property.h
+++ b/Framework/Kernel/inc/MantidKernel/Property.h
@@ -140,7 +140,10 @@ public:
"', property type not implemented.");
}
/// Returns the value of the property as a string
- virtual std::string value() const = 0;
+ virtual std::string value() const = 0;
+ /// Returns the value of the property as a pretty printed string
+ virtual std::string valuePrettyPrint(size_t maxLength = 0,
+ bool collapseLists = true) const;
/// Set the value of the property via a string. If the value is unacceptable
/// the value is not changed but a string is returned
virtual std::string setValue(const std::string &) = 0;
diff --git a/Framework/Kernel/inc/MantidKernel/PropertyHelper.h b/Framework/Kernel/inc/MantidKernel/PropertyHelper.h
index d8398ad9c93..5fb3fb4e241 100644
--- a/Framework/Kernel/inc/MantidKernel/PropertyHelper.h
+++ b/Framework/Kernel/inc/MantidKernel/PropertyHelper.h
@@ -28,13 +28,63 @@ template <typename T> std::string toString(const boost::shared_ptr<T> &value) {
throw boost::bad_lexical_cast();
}
+/// Specialization for a property of type std::vector.
+template <typename T>
+std::string toString(const std::vector<T> &value,
+ const std::string &delimiter = ",") {
+ std::stringstream result;
+ std::size_t vsize = value.size();
+ for (std::size_t i = 0; i < vsize; ++i) {
+ result << value[i];
+ if (i + 1 != vsize)
+ result << delimiter;
+ }
+ return result.str();
+}
+
+/// Specialization for a property of type std::vector<std::vector>.
+template <typename T>
+std::string toString(const std::vector<std::vector<T>> &value,
+ const std::string &outerDelimiter = ",",
+ const std::string &innerDelimiter = "+") {
+ std::stringstream result;
+ std::size_t vsize = value.size();
+ for (std::size_t i = 0; i < vsize; ++i) {
+ std::size_t innervsize = value[i].size();
+ for (std::size_t j = 0; j < innervsize; ++j) {
+ result << value[i][j];
+ if (j + 1 != innervsize)
+ result << innerDelimiter;
+ }
+
+ if (i + 1 != vsize)
+ result << outerDelimiter;
+ }
+ return result.str();
+}
+
+// --------------------- convert values to pretty strings
+/// Convert values to pretty strings.
+template <typename T> std::string toPrettyString(const T &value, size_t maxLength = 0,
+ bool collapseLists = true) {
+ return boost::lexical_cast<std::string>(value);
+}
+
+/// Throw an exception if a shared pointer is converted to a pretty string.
+template <typename T> std::string toPrettyString(const boost::shared_ptr<T> &value, size_t maxLength = 0,
+ bool collapseLists = true) {
+ UNUSED_ARG(value);
+ throw boost::bad_lexical_cast();
+}
+
/** Specialization for a property of type std::vector of non integral types.
* This will catch Vectors of char, double, float etc.
* This simply concatenates the values using a delimiter
*/
template <typename T>
std::string
-toString(const std::vector<T> &value, const std::string &delimiter = ",",
+toPrettyString(const std::vector<T> &value, size_t maxLength = 0,
+ bool collapseLists = true, const std::string &delimiter = ",",
const std::string &unusedDelimiter = "+",
typename std::enable_if<!(std::is_integral<T>::value &&
std::is_arithmetic<T>::value)>::type * = 0) {
@@ -51,7 +101,8 @@ toString(const std::vector<T> &value, const std::string &delimiter = ",",
*/
template <typename T>
std::string
-toString(const std::vector<T> &value, const std::string &delimiter = ",",
+toPrettyString(const std::vector<T> &value, size_t maxLength = 0,
+ bool collapseLists = true, const std::string &delimiter = ",",
const std::string &listDelimiter = "-",
typename std::enable_if<std::is_integral<T>::value &&
std::is_arithmetic<T>::value>::type * = 0) {
@@ -65,7 +116,8 @@ toString(const std::vector<T> &value, const std::string &delimiter = ",",
*/
template <>
std::string
-toString(const std::vector<bool> &value, const std::string &delimiter,
+toPrettyString(const std::vector<bool> &value, size_t maxLength,
+ bool collapseLists, const std::string &delimiter,
const std::string &unusedDelimiter,
typename std::enable_if<std::is_same<bool, bool>::value>::type *) {
UNUSED_ARG(unusedDelimiter);
@@ -74,23 +126,12 @@ toString(const std::vector<bool> &value, const std::string &delimiter,
/// Specialization for a property of type std::vector<std::vector>.
template <typename T>
-std::string toString(const std::vector<std::vector<T>> &value,
+std::string toPrettyString(const std::vector<std::vector<T>> &value,
+ size_t maxLength = 0,
+ bool collapseLists = true,
const std::string &outerDelimiter = ",",
const std::string &innerDelimiter = "+") {
- std::stringstream result;
- std::size_t vsize = value.size();
- for (std::size_t i = 0; i < vsize; ++i) {
- std::size_t innervsize = value[i].size();
- for (std::size_t j = 0; j < innervsize; ++j) {
- result << value[i][j];
- if (j + 1 != innervsize)
- result << innerDelimiter;
- }
-
- if (i + 1 != vsize)
- result << outerDelimiter;
- }
- return result.str();
+ return toString<T>(value, outerDelimiter, innerDelimiter);
}
/// Specialization for any type, should be appropriate for properties with a
diff --git a/Framework/Kernel/inc/MantidKernel/PropertyHistory.h b/Framework/Kernel/inc/MantidKernel/PropertyHistory.h
index 822073a13c5..4f300cce3ac 100644
--- a/Framework/Kernel/inc/MantidKernel/PropertyHistory.h
+++ b/Framework/Kernel/inc/MantidKernel/PropertyHistory.h
@@ -70,7 +70,7 @@ public:
/// get direction flag of algorithm parameter const
unsigned int direction() const { return m_direction; };
/// print contents of object
- void printSelf(std::ostream &, const int indent = 0) const;
+ void printSelf(std::ostream &, const int indent = 0, const size_t maxPropertyLength = 0) const;
/// get whether algorithm parameter was left as default EMPTY_INT,LONG,DBL
/// const
bool isEmptyDefault() const;
diff --git a/Framework/Kernel/inc/MantidKernel/PropertyWithValue.h b/Framework/Kernel/inc/MantidKernel/PropertyWithValue.h
index fab07ec92ba..d6f8684c2dd 100644
--- a/Framework/Kernel/inc/MantidKernel/PropertyWithValue.h
+++ b/Framework/Kernel/inc/MantidKernel/PropertyWithValue.h
@@ -66,6 +66,8 @@ public:
void saveProperty(::NeXus::File *file) override;
std::string value() const override;
+ std::string valuePrettyPrint(size_t maxLength = 0,
+ bool collapseLists = true) const;
virtual bool operator==(const PropertyWithValue<TYPE> &rhs) const;
virtual bool operator!=(const PropertyWithValue<TYPE> &rhs) const;
int size() const override;
diff --git a/Framework/Kernel/inc/MantidKernel/PropertyWithValue.tcc b/Framework/Kernel/inc/MantidKernel/PropertyWithValue.tcc
index fb0bdb27655..bd6bc0a5730 100644
--- a/Framework/Kernel/inc/MantidKernel/PropertyWithValue.tcc
+++ b/Framework/Kernel/inc/MantidKernel/PropertyWithValue.tcc
@@ -114,6 +114,14 @@ template <typename TYPE> std::string PropertyWithValue<TYPE>::value() const {
return toString(m_value);
}
+/** Get the value of the property as a string
+ * @return The property's value
+ */
+template <typename TYPE> std::string PropertyWithValue<TYPE>::valuePrettyPrint(size_t maxLength,
+ bool collapseLists) const const {
+ return toPrettyString(m_value, maxLength, collapseLists);
+}
+
/**
* Deep comparison.
* @param rhs The other property to compare to.
diff --git a/Framework/Kernel/inc/MantidKernel/Strings.h b/Framework/Kernel/inc/MantidKernel/Strings.h
index 086d992cce4..4cc570346d6 100644
--- a/Framework/Kernel/inc/MantidKernel/Strings.h
+++ b/Framework/Kernel/inc/MantidKernel/Strings.h
@@ -129,6 +129,9 @@ DLLExport std::string joinCompress(ITERATOR_TYPE begin, ITERATOR_TYPE end,
}
return result.str();
}
+/// Return a string with all matching occurence-strings
+MANTID_KERNEL_DLL std::string shortenList(const std::string &input,
+ const size_t &max_length);
/// Return a string with all matching occurence-strings
MANTID_KERNEL_DLL std::string replace(const std::string &input,
diff --git a/Framework/Kernel/src/Property.cpp b/Framework/Kernel/src/Property.cpp
index c64aa7ae412..bdd8b903835 100644
--- a/Framework/Kernel/src/Property.cpp
+++ b/Framework/Kernel/src/Property.cpp
@@ -4,6 +4,7 @@
#include "MantidKernel/Exception.h"
#include "MantidKernel/IPropertySettings.h"
#include "MantidKernel/PropertyHistory.h"
+#include "MantidKernel/Strings.h"
#include "MantidKernel/TimeSeriesProperty.h"
#include <unordered_map>
@@ -111,6 +112,18 @@ bool Property::remember() const { return m_remember; }
*/
void Property::setRemember(bool remember) { m_remember = remember; }
+/**
+* Returns the value as a pretty printed string
+* The default implementation just returns the value with the size limit applied
+* @param maxLength :: The Max length of the returned string
+* @param collapseLists :: Whether to collapse 1,2,3 into 1-3
+*/
+std::string Property::valuePrettyPrint(size_t maxLength,
+ bool collapseLists) const {
+ UNUSED_ARG(collapseLists);
+ return Strings::shortenList(value(),maxLength);
+}
+
/** Sets the user level description of the property.
* In addition, if the brief documentation string is empty it will be set to
* the portion of the provided string up to the first period
diff --git a/Framework/Kernel/src/PropertyHistory.cpp b/Framework/Kernel/src/PropertyHistory.cpp
index d63b06066e9..916eca6810f 100644
--- a/Framework/Kernel/src/PropertyHistory.cpp
+++ b/Framework/Kernel/src/PropertyHistory.cpp
@@ -4,6 +4,7 @@
#include "MantidKernel/EmptyValues.h"
#include "MantidKernel/PropertyHistory.h"
#include "MantidKernel/Property.h"
+#include "MantidKernel/Strings.h"
#include <boost/lexical_cast.hpp>
#include <algorithm>
@@ -22,28 +23,32 @@ PropertyHistory::PropertyHistory(const std::string &name,
m_direction(direction) {}
PropertyHistory::PropertyHistory(Property const *const prop)
- : // PropertyHistory::PropertyHistory(prop->name(), prop->value(),
- // prop->type(), prop->isDefault(), prop->direction())
- m_name(prop->name()),
- m_value(prop->value()), m_type(prop->type()),
+ : m_name(prop->name()),
+ m_value(prop->valuePrettyPrint(0,true)), m_type(prop->type()),
m_isDefault(prop->isDefault()), m_direction(prop->direction()) {}
/** Prints a text representation of itself
- * @param os :: The ouput stream to write to
+ * @param os :: The output stream to write to
* @param indent :: an indentation value to make pretty printing of object and
* sub-objects
+ * @param maxPropertyLength :: the max length for any property value string (0 = full length)
*/
-void PropertyHistory::printSelf(std::ostream &os, const int indent) const {
+void PropertyHistory::printSelf(std::ostream &os, const int indent,
+ const size_t maxPropertyLength) const {
os << std::string(indent, ' ') << "Name: " << m_name;
- os << ", Value: " << m_value;
+ if ((maxPropertyLength > 0) && (m_value.size() > maxPropertyLength)) {
+ os << ", Value: " << Strings::shortenList(m_value, maxPropertyLength);
+ } else {
+ os << ", Value: " << m_value;
+ }
os << ", Default?: " << (m_isDefault ? "Yes" : "No");
os << ", Direction: " << Kernel::Direction::asText(m_direction) << '\n';
}
/** Prints a text representation
- * @param os :: The ouput stream to write to
+ * @param os :: The output stream to write to
* @param AP :: The PropertyHistory to output
- * @returns The ouput stream
+ * @returns The output stream
*/
std::ostream &operator<<(std::ostream &os, const PropertyHistory &AP) {
AP.printSelf(os);
diff --git a/Framework/Kernel/src/Strings.cpp b/Framework/Kernel/src/Strings.cpp
index 80557bc268d..d304bea3862 100644
--- a/Framework/Kernel/src/Strings.cpp
+++ b/Framework/Kernel/src/Strings.cpp
@@ -37,6 +37,31 @@ std::string loadFile(const std::string &filename) {
return retVal;
}
+// ------------------------------------------------------------------------------------------------
+/** Return a string shortened with the center replace by " ... "
+* If the string is already short enough then the original string will be returned.
+* If the max length or input string length is smaller than the ellipsis, the input will be returned.
+*
+* @param input :: input string
+* @param max_length :: The maximum length of the return string (0 = return full string)
+* @return the modified string.
+*/
+std::string shortenList(const std::string & input, const size_t & max_length) {
+ const std::string ellipsis = " ... ";
+ const size_t ellipsisSize = ellipsis.size();
+ // limit too small or input too small, return input string
+ if ((max_length == 0) ||
+ (input.size() < ellipsisSize+2) ||
+ (input.size() <= max_length))
+ return input;
+
+ const size_t end_length = (max_length - ellipsisSize) / 2;
+ std::string retVal = input.substr(0, end_length) +
+ ellipsis +
+ input.substr(input.size() - end_length, end_length);
+ return retVal;
+}
+
//------------------------------------------------------------------------------------------------
/** Return a string with all matching occurence-strings
*
diff --git a/Framework/Kernel/test/ArrayPropertyTest.h b/Framework/Kernel/test/ArrayPropertyTest.h
index 7a14e268c7a..4914ee29058 100644
--- a/Framework/Kernel/test/ArrayPropertyTest.h
+++ b/Framework/Kernel/test/ArrayPropertyTest.h
@@ -82,7 +82,7 @@ public:
TS_ASSERT_EQUALS(i.operator()()[0], 1);
TS_ASSERT_EQUALS(i.operator()()[1], 2);
TS_ASSERT_EQUALS(i.operator()()[2], 3);
- TS_ASSERT_EQUALS(i.getDefault(), "1-3");
+ TS_ASSERT_EQUALS(i.getDefault(), "1,2,3");
TS_ASSERT(i.isDefault());
ArrayProperty<int> i2("i", "-1-1");
@@ -301,7 +301,7 @@ public:
static_cast<Property *>(0))
}
- void testListShortening() {
+ void testPrettyPrinting() {
std::vector<std::string> inputList{
"1,2,3", "-1,0,1", "356,366,367,368,370,371,372,375", "7,6,5,6,7,8,10",
"1-9998, 9999, 2000, 20002-29999"};
@@ -352,9 +352,10 @@ public:
bool success = true;
for (size_t i = 0; i < inputList.size(); i++) {
- ArrayProperty<T> intListProperty("i", inputList[i]);
- TS_ASSERT_EQUALS(intListProperty.value(), resultList[i]);
- if (intListProperty.value() != resultList[i]) {
+ ArrayProperty<T> listProperty("i", inputList[i]);
+ std::string response = listProperty.valuePrettyPrint(0,true);
+ TS_ASSERT_EQUALS(response, resultList[i]);
+ if (response != resultList[i]) {
success = false;
}
}
diff --git a/Framework/Kernel/test/PropertyHistoryTest.h b/Framework/Kernel/test/PropertyHistoryTest.h
index fa09873b510..519c592fd4c 100644
--- a/Framework/Kernel/test/PropertyHistoryTest.h
+++ b/Framework/Kernel/test/PropertyHistoryTest.h
@@ -28,6 +28,21 @@ public:
TS_ASSERT_EQUALS(output.str(), correctOutput);
}
+ void testOutputWithShortenedValue() {
+ std::string correctOutput = "Name: arg1_param, ";
+ correctOutput = correctOutput + "Value: 1234567 ... 4567890, ";
+ correctOutput = correctOutput + "Default?: Yes, ";
+ correctOutput = correctOutput + "Direction: Input\n";
+
+ // a long property that should get shortened
+ PropertyHistory propHistory("arg1_param", "123456798012345678901234567890", "argument", true, Direction::Input);
+
+ // dump output to sting
+ std::ostringstream output;
+ TS_ASSERT_THROWS_NOTHING(propHistory.printSelf(output,0,20));
+ TS_ASSERT_EQUALS(output.str(), correctOutput);
+ }
+
/**
* Test the isEmptyDefault method returns true for unset default-value
* properties
diff --git a/Framework/Kernel/test/StringsTest.h b/Framework/Kernel/test/StringsTest.h
index f6429f13cd1..4e42b24cab8 100644
--- a/Framework/Kernel/test/StringsTest.h
+++ b/Framework/Kernel/test/StringsTest.h
@@ -285,6 +285,69 @@ public:
}
}
+ void test_shortenList() {
+
+ std::vector<std::string> inputList{
+ "", // empty
+ "1,2", // shorter than the ellipsis
+ "1,2,3", // equal in length than the ellipsis
+ "1,2,35", // one longer than the ellipsis
+ "1,2,3,4", // two longer than the ellipsis
+ "1,2,3,45", // just long enough for the ellipsis
+ "12,3,4,56", // another past the ellipsis
+ "1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20" };
+ std::vector<std::string> resultListMaxLength7{
+ "", // empty
+ "1,2", // shorter than the ellipsis
+ "1,2,3", // equal in length than the ellipsis
+ "1,2,35", // one longer than the ellipsis
+ "1,2,3,4", // two longer than the ellipsis
+ "1 ... 5", // just long enough for the ellipsis
+ "1 ... 6", // another past the ellipsis
+ "1 ... 0" };
+
+ std::vector<std::string> resultListMaxLength20{
+ "", // empty
+ "1,2", // shorter than the ellipsis
+ "1,2,3", // equal in length than the ellipsis
+ "1,2,35", // one longer than the ellipsis
+ "1,2,3,4", // two longer than the ellipsis
+ "1,2,3,45", // just long enough for the ellipsis
+ "12,3,4,56", // another past the ellipsis
+ "1,2,3,4 ... 8,19,20" };
+
+ //test very short max size
+ int maxLength = 7;
+ for (size_t i = 0; i < inputList.size(); i++) {
+ const auto &input = inputList[i];
+ std::string result = shortenList(input, maxLength);
+ TS_ASSERT_EQUALS(
+ result,
+ resultListMaxLength7[i]);
+ TS_ASSERT_LESS_THAN_EQUALS(
+ result.size(),
+ maxLength);
+ TS_ASSERT_LESS_THAN_EQUALS(
+ result.size(),
+ input.size());
+ }
+ //test longer max size
+ maxLength = 20;
+ for (size_t i = 0; i < inputList.size(); i++) {
+ const auto &input = inputList[i];
+ std::string result = shortenList(input, maxLength);
+ TS_ASSERT_EQUALS(
+ result,
+ resultListMaxLength20[i]);
+ TS_ASSERT_LESS_THAN_EQUALS(
+ result.size(),
+ maxLength);
+ TS_ASSERT_LESS_THAN_EQUALS(
+ result.size(),
+ input.size());
+ }
+ }
+
void test_endsWithInt() {
TS_ASSERT_EQUALS(endsWithInt("pixel22"), 22);
TS_ASSERT_EQUALS(endsWithInt("pixel000123"), 123);
--
GitLab