LoadNexusProcessed: root-level NXentry groups

Framework/DataHandling/src/LoadNexusProcessed.cpp

@@ -40,6 +40,7 @@
 #include <boost/regex.hpp>
 #include <nexus/NeXusException.hpp>
 
+#include <algorithm>
 #include <map>
 #include <memory>
 #include <string>
@@ -373,28 +374,28 @@ Workspace_sptr LoadNexusProcessed::doAccelleratedMultiPeriodLoading(NXRoot &root
 void LoadNexusProcessed::execLoader() {
 
   API::Workspace_sptr tempWS;
-  int nWorkspaceEntries = 0;
+  size_t nWorkspaceEntries = 0;
   // Start scoped block
   {
     progress(0, "Opening file...");
 
-    // Throws an approriate exception if there is a problem with file access
+    // Throws an appropriate exception if there is a problem with file access
     const std::string filename = getPropertyValue("Filename");
     NXRoot root(filename);
 
     // "Open" the same file but with the C++ interface
     m_nexusFile = std::make_unique<::NeXus::File>(root.m_fileID);
 
-    // Find out how many first level entries there are
-    // Cast down to int as another property later on is an int
-    nWorkspaceEntries = static_cast<int>((root.groups().size()));
+    // Find out how many NXentry groups there are in the file.
+    nWorkspaceEntries = std::count_if(root.groups().cbegin(), root.groups().cend(),
+                                      [](const auto &g) { return g.nxclass == "NXentry"; });
 
     // Check for an entry number property
     int entrynumber = getProperty("EntryNumber");
     Property const *const entryNumberProperty = this->getProperty("EntryNumber");
     bool bDefaultEntryNumber = entryNumberProperty->isDefault();
 
-    if (!bDefaultEntryNumber && entrynumber > nWorkspaceEntries) {
+    if (!bDefaultEntryNumber && entrynumber > static_cast<int>(nWorkspaceEntries)) {
       g_log.error() << "Invalid entry number specified. File only contains " << nWorkspaceEntries << " entries.\n";
       throw std::invalid_argument("Invalid entry number specified.");
     }
@@ -421,7 +422,7 @@ void LoadNexusProcessed::execLoader() {
       // We already know that this is a group workspace. Is it a true
       // multiperiod workspace.
       const bool bFastMultiPeriod = this->getProperty("FastMultiPeriod");
-      const bool bIsMultiPeriod = isMultiPeriodFile(nWorkspaceEntries, tempWS, g_log);
+      const bool bIsMultiPeriod = isMultiPeriodFile(static_cast<int>(nWorkspaceEntries), tempWS, g_log);
       const Property *specListProp = this->getProperty("SpectrumList");
       m_list = !specListProp->isDefault();
 
@@ -433,7 +434,7 @@ void LoadNexusProcessed::execLoader() {
 
       // load names of each of the workspaces. Note that if we have duplicate
       // names then we don't select them
-      auto names = extractWorkspaceNames(root, static_cast<size_t>(nWorkspaceEntries));
+      auto names = extractWorkspaceNames(root, nWorkspaceEntries);
 
       // remove existing workspace and replace with the one being loaded
       bool wsExists = AnalysisDataService::Instance().doesExist(base_name);
@@ -465,7 +466,7 @@ void LoadNexusProcessed::execLoader() {
         g_log.information("Individual group loading");
       }
 
-      for (int p = 1; p <= nWorkspaceEntries; ++p) {
+      for (size_t p = 1; p <= nWorkspaceEntries; ++p) {
         const auto indexStr = std::to_string(p);
 
         // decide what the workspace should be called
@@ -503,7 +504,7 @@ void LoadNexusProcessed::execLoader() {
     root.close();
   } // All file resources should be scoped to here. All previous file handles
   // must be cleared to release locks
-  loadNexusGeometry(*tempWS, nWorkspaceEntries, g_log, std::string(getProperty("Filename")));
+  loadNexusGeometry(*tempWS, static_cast<int>(nWorkspaceEntries), g_log, std::string(getProperty("Filename")));
 
   m_axis1vals.clear();
 } // namespace DataHandling
@@ -913,13 +914,13 @@ void LoadNexusProcessed::loadV3DColumn(Mantid::NeXus::NXDouble &data, const API:
 
     const int rowCount = data.dim0();
 
-    // This might've been done already, but doing it twice should't do any harm
+    // This might've been done already, but doing it twice shouldn't do any harm
     tableWs->setRowCount(rowCount);
 
     data.load();
 
     for (int i = 0; i < rowCount; ++i) {
-      auto &cell = col[i];
+      auto &cell = col[i]; // cppcheck-suppress constVariableReference
       cell(data(i, 0), data(i, 1), data(i, 2));
     }
   }

Framework/DataHandling/test/LoadNexusProcessedTest.h

@@ -539,6 +539,34 @@ class LoadNexusProcessedTest : public CxxTest::TestSuite {
     }
   }
 
+  void test_load_workspace_group_other_root_groups() {
+
+    // Test that a group workspace can be loaded in the presence of other
+    //   non-NXentry NeXus and non-NeXus root groups.
+
+    LoadNexusProcessed alg;
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
+    alg.setPropertyValue("Filename", "WorkspaceGroup_other_groups.nxs");
+    alg.setPropertyValue("OutputWorkspace", "group");
+
+    TS_ASSERT_THROWS_NOTHING(alg.execute());
+
+    Workspace_sptr workspace;
+    TS_ASSERT_THROWS_NOTHING(workspace = AnalysisDataService::Instance().retrieve("group"));
+    WorkspaceGroup_sptr group = std::dynamic_pointer_cast<WorkspaceGroup>(workspace);
+    TS_ASSERT(group);
+    int groupSize = group->getNumberOfEntries();
+    TS_ASSERT_EQUALS(groupSize, 12);
+    for (int i = 0; i < groupSize; ++i) {
+      MatrixWorkspace_sptr ws = std::dynamic_pointer_cast<MatrixWorkspace>(group->getItem(i));
+      TS_ASSERT(ws);
+      TS_ASSERT_EQUALS(ws->getNumberHistograms(), 1);
+      TS_ASSERT_EQUALS(ws->blocksize(), 10);
+      TS_ASSERT_EQUALS(ws->getName(), "group_" + std::to_string(i + 1));
+    }
+  }
+
   void test_load_fit_parameters() {
     LoadNexusProcessed alg;
     TS_ASSERT_THROWS_NOTHING(alg.initialize());

Testing/Data/UnitTest/WorkspaceGroup_other_groups.nxs.md5

@@ -0,0 +1 @@
+cc76a6f1e3a36626a1d38cda0f88e5ac

buildconfig/CMake/CppCheck_Suppressions.txt.in

@@ -733,7 +733,6 @@ constVariableReference:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveFullpr
 passedByValue:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:579
 passedByValue:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:580
 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:158
-constVariableReference:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/LoadNexusProcessed.cpp:922
 containerOutOfBounds:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:607
 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveLauenorm.cpp:164
 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveLauenorm.cpp:234

docs/source/release/v6.12.0/Framework/Algorithms/Bugfixes/38324.rst

@@ -0,0 +1 @@
+- fixes a bug in :ref:`LoadNexusProcessed <algm-LoadNexusProcessed>` when determining the number of workspaces in a NeXus HDF5 file.  It now counts the number of root-level "NX_class: NXentry" groups. Previously, it simply counted the number of root-level groups, assuming all were of "NX_class: NXentry".