From 8a446fb1fbcaea0aa636562bc70eaec672253546 Mon Sep 17 00:00:00 2001
From: Robert Applin <40830825+robertapplin@users.noreply.github.com>
Date: Wed, 26 Sep 2018 14:25:53 +0100
Subject: [PATCH] Fix combinations of spectra from two datasets Refs #23463

---
 .../Indirect/IndirectFitData.cpp              | 65 +++++++++++----
 .../IndirectSpectrumSelectionPresenter.cpp    | 21 ++++-
 .../Indirect/test/IndirectFitDataTest.h       | 81 +++++++++++++++++--
 3 files changed, 145 insertions(+), 22 deletions(-)

diff --git a/qt/scientific_interfaces/Indirect/IndirectFitData.cpp b/qt/scientific_interfaces/Indirect/IndirectFitData.cpp
index b746ac44bf8..4d93be08dbe 100644
--- a/qt/scientific_interfaces/Indirect/IndirectFitData.cpp
+++ b/qt/scientific_interfaces/Indirect/IndirectFitData.cpp
@@ -1,5 +1,7 @@
 #include "IndirectFitData.h"
 
+#include "MantidKernel/Strings.h"
+
 #include <boost/algorithm/string.hpp>
 #include <boost/format.hpp>
 
@@ -9,6 +11,17 @@ using namespace Mantid::API;
 
 namespace {
 using namespace MantidQt::CustomInterfaces::IDA;
+using namespace Mantid::Kernel::Strings;
+
+std::vector<std::size_t>
+convertStringVectorToSizeT(std::vector<std::string> const &vec) {
+  std::vector<std::size_t> newVec;
+  newVec.reserve(vec.size());
+  for (auto element : vec)
+    if (!element.empty())
+      newVec.emplace_back(std::stoull(element));
+  return newVec;
+}
 
 std::string rangeToString(const std::pair<std::size_t, std::size_t> &range,
                           const std::string &delimiter = "-") {
@@ -90,6 +103,21 @@ private:
   const std::string m_rangeDelimiter;
 };
 
+std::string
+constructDiscontinuousSpectraString(std::vector<int> const &spectras) {
+  return joinCompress(spectras.begin(), spectras.end());
+}
+
+std::string createDiscontinuousSpectraString(std::string const &string) {
+  std::string spectraString = string;
+  std::remove_if(spectraString.begin(), spectraString.end(), isspace);
+  std::vector<int> spectras = parseRange(spectraString);
+  std::sort(spectras.begin(), spectras.end());
+  // Remove duplicate entries
+  spectras.erase(std::unique(spectras.begin(), spectras.end()), spectras.end());
+  return constructDiscontinuousSpectraString(spectras);
+}
+
 struct CombineSpectra : boost::static_visitor<Spectra> {
   Spectra
   operator()(const std::pair<std::size_t, std::size_t> &spectra1,
@@ -98,15 +126,16 @@ struct CombineSpectra : boost::static_visitor<Spectra> {
       return std::make_pair(spectra1.first, spectra2.second);
     else if (spectra2.second + 1 == spectra1.first)
       return std::make_pair(spectra2.first, spectra1.second);
-    else
-      return DiscontinuousSpectra<std::size_t>(rangeToString(spectra1) + "," +
-                                               rangeToString(spectra2));
+    else {
+      return DiscontinuousSpectra<std::size_t>(createDiscontinuousSpectraString(
+          rangeToString(spectra1) + "," + rangeToString(spectra2)));
+    }
   }
 
   Spectra operator()(const Spectra &spectra1, const Spectra &spectra2) const {
-    return DiscontinuousSpectra<std::size_t>(
+    return DiscontinuousSpectra<std::size_t>(createDiscontinuousSpectraString(
         boost::apply_visitor(SpectraToString(), spectra1) + "," +
-        boost::apply_visitor(SpectraToString(), spectra2));
+        boost::apply_visitor(SpectraToString(), spectra2)));
   }
 };
 
@@ -194,13 +223,19 @@ std::string orderExcludeRegionString(std::vector<double> &bounds) {
   return constructExcludeRegionString(bounds);
 }
 
-std::string arrangeExcludeRegionString(std::string const &excludeRegionString) {
-  auto boundStrings = splitStringBy(excludeRegionString, ", ");
-
+std::vector<double>
+getBoundsAsDoubleVector(std::vector<std::string> const &boundStrings) {
   std::vector<double> bounds;
+  bounds.reserve(boundStrings.size());
   for (auto bound : boundStrings)
-    if (!bound.empty())
-      bounds.emplace_back(convertBoundToDoubleAndFormat(bound));
+    bounds.emplace_back(convertBoundToDoubleAndFormat(bound));
+  return bounds;
+}
+
+std::string createExcludeRegionString(std::string const &string) {
+  std::string regionString = string;
+  std::remove_if(regionString.begin(), regionString.end(), isspace);
+  auto bounds = getBoundsAsDoubleVector(splitStringBy(regionString, ","));
   return orderExcludeRegionString(bounds);
 }
 
@@ -282,9 +317,10 @@ IndirectFitData::excludeRegionsVector(std::size_t spectrum) const {
   return vectorFromString<double>(getExcludeRegion(spectrum));
 }
 
-void IndirectFitData::setSpectra(const std::string &spectra) {
+void IndirectFitData::setSpectra(std::string const &spectra) {
   try {
-    const Spectra spec = DiscontinuousSpectra<std::size_t>(spectra);
+    const Spectra spec = DiscontinuousSpectra<std::size_t>(
+        createDiscontinuousSpectraString(spectra));
     setSpectra(spec);
   } catch (std::exception &ex) {
     throw std::runtime_error("Spectra too large for cast: " +
@@ -297,7 +333,7 @@ void IndirectFitData::setSpectra(Spectra &&spectra) {
   m_spectra = std::move(spectra);
 }
 
-void IndirectFitData::setSpectra(const Spectra &spectra) {
+void IndirectFitData::setSpectra(Spectra const &spectra) {
   validateSpectra(spectra);
   m_spectra = spectra;
 }
@@ -339,8 +375,7 @@ void IndirectFitData::setEndX(double const &endX, std::size_t const &spectrum) {
 void IndirectFitData::setExcludeRegionString(
     std::string const &excludeRegionString, std::size_t const &spectrum) {
   if (!excludeRegionString.empty())
-    m_excludeRegions[spectrum] =
-        arrangeExcludeRegionString(excludeRegionString);
+    m_excludeRegions[spectrum] = createExcludeRegionString(excludeRegionString);
   else
     m_excludeRegions[spectrum] = excludeRegionString;
 }
diff --git a/qt/scientific_interfaces/Indirect/IndirectSpectrumSelectionPresenter.cpp b/qt/scientific_interfaces/Indirect/IndirectSpectrumSelectionPresenter.cpp
index e15a15625f7..2e917f0b316 100644
--- a/qt/scientific_interfaces/Indirect/IndirectSpectrumSelectionPresenter.cpp
+++ b/qt/scientific_interfaces/Indirect/IndirectSpectrumSelectionPresenter.cpp
@@ -1,7 +1,7 @@
 #include "IndirectSpectrumSelectionPresenter.h"
 
 #include "MantidKernel/ArrayProperty.h"
-
+#include "MantidKernel/Strings.h"
 #include "MantidQtWidgets/Common/SignalBlocker.h"
 
 #include <algorithm>
@@ -12,6 +12,7 @@
 
 namespace {
 using namespace MantidQt::CustomInterfaces::IDA;
+using namespace Mantid::Kernel::Strings;
 
 struct SetViewSpectra : boost::static_visitor<> {
   explicit SetViewSpectra(IndirectSpectrumSelectionView *view) : m_view(view) {}
@@ -37,6 +38,21 @@ std::string NATURAL_NUMBER(std::size_t digits) {
   return OR("0", "[1-9][0-9]{," + std::to_string(digits - 1) + "}");
 }
 
+std::string
+constructDiscontinuousSpectraString(std::vector<int> const &spectras) {
+  return joinCompress(spectras.begin(), spectras.end());
+}
+
+std::string createDiscontinuousSpectraString(std::string const &string) {
+  std::string spectraString = string;
+  std::remove_if(spectraString.begin(), spectraString.end(), isspace);
+  std::vector<int> spectras = parseRange(spectraString);
+  std::sort(spectras.begin(), spectras.end());
+  // Remove duplicate entries
+  spectras.erase(std::unique(spectras.begin(), spectras.end()), spectras.end());
+  return constructDiscontinuousSpectraString(spectras);
+}
+
 namespace Regexes {
 const std::string EMPTY = "^$";
 const std::string SPACE = "[ ]*";
@@ -145,7 +161,8 @@ void IndirectSpectrumSelectionPresenter::setModelSpectra(
 
 void IndirectSpectrumSelectionPresenter::updateSpectraList(
     const std::string &spectraList) {
-  setModelSpectra(DiscontinuousSpectra<std::size_t>(spectraList));
+  setModelSpectra(DiscontinuousSpectra<std::size_t>(
+      createDiscontinuousSpectraString(spectraList)));
   emit spectraChanged(m_activeIndex);
 }
 
diff --git a/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h b/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h
index c235124a462..b6aaf6d0203 100644
--- a/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h
+++ b/qt/scientific_interfaces/Indirect/test/IndirectFitDataTest.h
@@ -335,7 +335,7 @@ public:
   }
 
   void
-  test_that_the_spectra_of_two_datasets_are_combined_correctly_when_spectra_do_not_overlap() {
+  test_that_the_spectra_pair_of_two_datasets_are_combined_correctly_when_spectra_do_not_overlap() {
     auto data1 = getIndirectFitData(10, 3);
     auto data2 = getIndirectFitData(10, 3);
 
@@ -349,14 +349,85 @@ public:
   }
 
   void
-  test_that_the_spectra_of_two_datasets_with_discontinous_spectra_are_combined_correctly_when_spectra_do_not_overlap() {
+  test_that_the_spectra_pair_of_two_datasets_are_combined_correctly_when_spectra_are_discontinuous() {
     auto data1 = getIndirectFitData(10, 3);
     auto data2 = getIndirectFitData(10, 3);
 
-    data1.setSpectra(DiscontinuousSpectra<std::size_t>("1,4"));
-    data2.setSpectra(DiscontinuousSpectra<std::size_t>("5,9"));
+    data1.setSpectra(std::make_pair(0u, 4u));
+    data2.setSpectra(std::make_pair(8u, 9u));
+    auto combinedData = data2.combine(data1);
+    Spectra spec(DiscontinuousSpectra<std::size_t>("0-4,8-9"));
+
+    TS_ASSERT(
+        boost::apply_visitor(AreSpectraEqual(), combinedData.spectra(), spec));
+  }
+
+  void
+  test_that_the_spectra_pair_of_two_datasets_are_combined_correctly_when_spectra_overlap() {
+    auto data1 = getIndirectFitData(10, 3);
+    auto data2 = getIndirectFitData(10, 3);
+
+    data1.setSpectra(std::make_pair(0u, 8u));
+    data2.setSpectra(std::make_pair(4u, 9u));
+    auto combinedData = data2.combine(data1);
+    // Spectra spec(std::make_pair(0u, 9u));
+    Spectra spec(DiscontinuousSpectra<std::size_t>("0-9"));
+
+    TS_ASSERT(
+        boost::apply_visitor(AreSpectraEqual(), combinedData.spectra(), spec));
+  }
+
+  void
+  test_that_the_DiscontinousSpectra_of_two_datasets_are_combined_correctly_when_spectra_do_not_overlap() {
+    auto data1 = getIndirectFitData(10, 3);
+    auto data2 = getIndirectFitData(10, 3);
+
+    data1.setSpectra(DiscontinuousSpectra<std::size_t>("0-4"));
+    data2.setSpectra(DiscontinuousSpectra<std::size_t>("5-9"));
+    auto combinedData = data2.combine(data1);
+    Spectra spec(DiscontinuousSpectra<std::size_t>("0-9"));
+
+    TS_ASSERT(
+        boost::apply_visitor(AreSpectraEqual(), combinedData.spectra(), spec));
+  }
+
+  void
+  test_that_the_DiscontinousSpectra_of_two_datasets_are_combined_correctly_when_spectra_overlap() {
+    auto data1 = getIndirectFitData(10, 3);
+    auto data2 = getIndirectFitData(10, 3);
+
+    data1.setSpectra(DiscontinuousSpectra<std::size_t>("0-7"));
+    data2.setSpectra(DiscontinuousSpectra<std::size_t>("2-9"));
+    auto combinedData = data2.combine(data1);
+    Spectra spec(DiscontinuousSpectra<std::size_t>("0-9"));
+
+    TS_ASSERT(
+        boost::apply_visitor(AreSpectraEqual(), combinedData.spectra(), spec));
+  }
+
+  void
+  test_that_a_SpectraPair_and_DiscontinousSpectra_dataset_are_combined_correctly_when_spectra_do_not_overlap() {
+    auto data1 = getIndirectFitData(10, 3);
+    auto data2 = getIndirectFitData(10, 3);
+
+    data1.setSpectra(DiscontinuousSpectra<std::size_t>("0-4"));
+    data2.setSpectra(std::make_pair(5u, 9u));
+    auto combinedData = data2.combine(data1);
+    Spectra spec(DiscontinuousSpectra<std::size_t>("0-9"));
+
+    TS_ASSERT(
+        boost::apply_visitor(AreSpectraEqual(), combinedData.spectra(), spec));
+  }
+
+  void
+  test_that_a_SpectraPair_and_DiscontinousSpectra_dataset_are_combined_correctly_when_spectra_overlap() {
+    auto data1 = getIndirectFitData(10, 3);
+    auto data2 = getIndirectFitData(10, 3);
+
+    data1.setSpectra(DiscontinuousSpectra<std::size_t>("0-7"));
+    data2.setSpectra(std::make_pair(4u, 9u));
     auto combinedData = data2.combine(data1);
-    Spectra spec(DiscontinuousSpectra<std::size_t>("5,9,1,4"));
+    Spectra spec(DiscontinuousSpectra<std::size_t>("0-9"));
 
     TS_ASSERT(
         boost::apply_visitor(AreSpectraEqual(), combinedData.spectra(), spec));
-- 
GitLab