Remove isReadable/HDFVersion from NexusHDF5Descriptor and try/catch instead

Framework/API/src/FileLoaderRegistry.cpp

@@ -8,6 +8,7 @@
 #include "MantidAPI/IFileLoader.h"
 #include "MantidAPI/NexusFileLoader.h"
 
+#include <H5Cpp.h>
 #include <Poco/File.h>
 
 namespace Mantid::API {
@@ -117,21 +118,29 @@ const std::shared_ptr<IAlgorithm> FileLoaderRegistryImpl::chooseLoader(const std
   m_log.debug() << "Trying to find loader for '" << filename << "'\n";
 
   IAlgorithm_sptr bestLoader;
-  auto HDFversion = NexusHDF5Descriptor::getHDFVersion(filename);
-  if (HDFversion == NexusHDF5Descriptor::Version5) {
+
+  if (H5::H5File::isHdf5(filename)) {
     std::pair<IAlgorithm_sptr, int> HDF5result =
         searchForLoader<NexusHDF5Descriptor, IFileLoader<NexusHDF5Descriptor>>(filename, m_names[NexusHDF5], m_log);
 
     // must also try NexusDescriptor algorithms because LoadMuonNexus can load both HDF4 and HDF5 files
     std::pair<IAlgorithm_sptr, int> HDF4result =
         searchForLoader<NexusDescriptor, IFileLoader<NexusDescriptor>>(filename, m_names[Nexus], m_log);
+
     if (HDF5result.second > HDF4result.second)
       bestLoader = HDF5result.first;
     else
       bestLoader = HDF4result.first;
-  } else if (HDFversion == NexusHDF5Descriptor::Version4)
-    bestLoader = searchForLoader<NexusDescriptor, IFileLoader<NexusDescriptor>>(filename, m_names[Nexus], m_log).first;
-  else
+  } else {
+    try {
+      bestLoader =
+          searchForLoader<NexusDescriptor, IFileLoader<NexusDescriptor>>(filename, m_names[Nexus], m_log).first;
+    } catch (std::exception const &e) {
+      m_log.debug() << "Error in looking for NeXus files: " << e.what() << '\n';
+    }
+  }
+
+  if (!bestLoader)
     bestLoader = searchForLoader<FileDescriptor, IFileLoader<FileDescriptor>>(filename, m_names[Generic], m_log).first;
 
   if (!bestLoader) {
@@ -165,11 +174,13 @@ bool FileLoaderRegistryImpl::canLoad(const std::string &algorithmName, const std
   std::multimap<std::string, int> names{{algorithmName, -1}};
   IAlgorithm_sptr loader;
   if (nexus) {
-    if (NexusHDF5Descriptor::isReadable(filename, NexusHDF5Descriptor::Version4)) {
+    try {
       loader = searchForLoader<NexusDescriptor, IFileLoader<NexusDescriptor>>(filename, names, m_log).first;
+    } catch (std::exception const &e) {
+      m_log.debug() << "Error in looking for NeXus files: " << e.what() << '\n';
     }
   } else if (nexusHDF5) {
-    if (NexusHDF5Descriptor::isReadable(filename)) {
+    if (H5::H5File::isHdf5(filename)) {
       try {
         loader = searchForLoader<NexusHDF5Descriptor, IFileLoader<NexusHDF5Descriptor>>(filename, names, m_log).first;
       } catch (const std::invalid_argument &e) {

Framework/DataHandling/src/LoadGeometry.cpp

@@ -26,7 +26,7 @@ bool LoadGeometry::isNexus(const std::string &filename) {
     return false;
   }
 
-  if (Mantid::Kernel::NexusHDF5Descriptor::isReadable(filename)) {
+  if (H5::H5File::isHdf5(filename)) {
     Mantid::Kernel::NexusHDF5Descriptor descriptor(filename);
     return isNexus(descriptor.getAllEntries());
   }

Framework/DataHandling/src/UpdateInstrumentFromFile.cpp

@@ -20,6 +20,7 @@
 #include "MantidNexusCpp/NeXusException.hpp"
 #include "MantidNexusCpp/NeXusFile.hpp"
 
+#include <H5Cpp.h>
 #include <algorithm>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/algorithm/string/trim.hpp>
@@ -88,7 +89,7 @@ void UpdateInstrumentFromFile::exec() {
   m_ignoreMonitors = (!moveMonitors);
 
   // Check file type
-  if (NexusHDF5Descriptor::isReadable(filename)) {
+  if (H5::H5File::isHdf5(filename)) {
     LoadISISNexus2 isisNexus;
     LoadEventNexus eventNexus;
 

Framework/DataHandling/test/LoadTOFRawNexusTest.h

@@ -45,10 +45,6 @@ class LoadTOFRawNexusTest : public CxxTest::TestSuite {
     Mantid::Kernel::NexusHDF5Descriptor descr2(alg.getPropertyValue("Filename"));
     TS_ASSERT_EQUALS(alg.confidence(descr2), 20);
 
-    alg.setPropertyValue("Filename", "argus0026577.nxs");
-    Mantid::Kernel::NexusHDF5Descriptor descr3(alg.getPropertyValue("Filename"));
-    TS_ASSERT_EQUALS(alg.confidence(descr3), 0);
-
     alg.setPropertyValue("Filename", "PG3_733.nxs");
     Mantid::Kernel::NexusHDF5Descriptor descr4(alg.getPropertyValue("Filename"));
     TS_ASSERT_EQUALS(alg.confidence(descr4), 0);

Framework/Kernel/inc/MantidKernel/NexusDescriptor.h

@@ -33,7 +33,7 @@ class MANTID_KERNEL_DLL NexusDescriptor {
 
 public:
   /// Constructor accepting a filename
-  NexusDescriptor(const std::string &filename, const bool init = true);
+  NexusDescriptor(const std::string &filename);
   /// Destructor
   ~NexusDescriptor();
 

Framework/Kernel/inc/MantidKernel/NexusHDF5Descriptor.h

@@ -20,14 +20,6 @@ namespace Kernel {
 class MANTID_KERNEL_DLL NexusHDF5Descriptor {
 
 public:
-  /// Enumerate HDF possible versions
-  enum Version { Version4, Version5, None };
-
-  /// Returns true if the file is considered to store data in a hierarchy
-  static bool isReadable(const std::string &filename, const Version version = Version5);
-  /// Returns version of HDF file
-  static Version getHDFVersion(const std::string &filename);
-
   /**
    * Unique constructor
    * @param filename input HDF5 Nexus file name

Framework/Kernel/src/NexusDescriptor.cpp

@@ -21,29 +21,25 @@ namespace Mantid::Kernel {
 /**
  * Constructs the wrapper
  * @param filename A string pointing to an existing file
- * @param init Whether or not to initialize the file
  * @throws std::invalid_argument if the file is not identified to be
  * hierarchical. This currently
  * involves simply checking for the signature if a HDF file at the start of the
  * file
  */
-NexusDescriptor::NexusDescriptor(const std::string &filename, const bool init)
+NexusDescriptor::NexusDescriptor(const std::string &filename)
     : m_filename(), m_extension(), m_firstEntryNameType(), m_rootAttrs(), m_pathsToTypes(), m_file(nullptr) {
   if (filename.empty()) {
     throw std::invalid_argument("NexusDescriptor() - Empty filename '" + filename + "'");
   }
   if (!std::filesystem::exists(filename)) {
     throw std::invalid_argument("NexusDescriptor() - File '" + filename + "' does not exist");
   }
-
-  if (init) {
-    try {
-      // this is very expesive as it walk the entire file
-      initialize(filename);
-    } catch (::NeXus::Exception &e) {
-      throw std::invalid_argument("NexusDescriptor::initialize - File '" + filename +
-                                  "' does not look like a HDF file.\n Error was: " + e.what());
-    }
+  try {
+    // this is very expesive as it walk the entire file
+    initialize(filename);
+  } catch (::NeXus::Exception &e) {
+    throw std::invalid_argument("NexusDescriptor::initialize - File '" + filename +
+                                "' does not look like a HDF file.\n Error was: " + e.what());
   }
 }
 

Framework/Kernel/src/NexusHDF5Descriptor.cpp

@@ -24,46 +24,6 @@ namespace Mantid::Kernel {
 /// PRIVATE
 namespace {
 
-/// Size of HDF magic number
-const size_t HDFMagicSize = 4;
-/// HDF cookie that is stored in the first 4 bytes of the file.
-const unsigned char HDFMagic[4] = {'\016', '\003', '\023', '\001'}; // From HDF4::hfile.h
-
-/// Size of HDF5 signature
-const size_t HDF5SignatureSize = 8;
-/// signature identifying a HDF5 file.
-const unsigned char HDF5Signature[8] = {137, 'H', 'D', 'F', '\r', '\n', '\032', '\n'};
-//---------------------------------------------------------------------------------------------------------------------------
-// Anonymous helper methods to use isReadable methods to use an open file handle
-//---------------------------------------------------------------------------------------------------------------------------
-
-NexusHDF5Descriptor::Version HDFversion(FILE *fileHandle) {
-  if (!fileHandle)
-    throw std::invalid_argument("HierarchicalFileDescriptor::isHierarchical - Invalid file handle");
-
-  // HDF4 check requires 4 bytes,  HDF5 check requires 8 bytes
-  // Use same buffer and waste a few bytes if only checking HDF4
-  unsigned char buffer[8] = {'0', '0', '0', '0', '0', '0', '0', '0'};
-  if (HDF5SignatureSize !=
-      std::fread(static_cast<void *>(&buffer), sizeof(unsigned char), HDF5SignatureSize, fileHandle)) {
-    throw std::runtime_error("Error while reading file");
-  }
-
-  NexusHDF5Descriptor::Version result = NexusHDF5Descriptor::None;
-
-  // Number of bytes read doesn't matter as if it is not enough then the memory
-  // simply won't match
-  // as the buffer has been "zeroed"
-  if (std::memcmp(&buffer, &HDF5Signature, HDF5SignatureSize) == 0)
-    result = NexusHDF5Descriptor::Version5;
-  else if (std::memcmp(&buffer, &HDFMagic, HDFMagicSize) == 0)
-    result = NexusHDF5Descriptor::Version4;
-
-  // Return file stream to start of file
-  std::rewind(fileHandle);
-  return result;
-}
-
 void getGroup(H5::Group groupID, std::map<std::string, std::set<std::string>> &allEntries,
               std::pair<std::string, std::string> &firstEntryNameType, size_t level) {
 
@@ -107,31 +67,6 @@ void getGroup(H5::Group groupID, std::map<std::string, std::set<std::string>> &a
 }
 } // namespace
 
-//---------------------------------------------------------------------------------------------------------------------------
-// static NexusHDF5Descriptor methods
-//---------------------------------------------------------------------------------------------------------------------------
-
-/**
- * Checks for the HDF signatures and returns true if one of them is found
- * @param filename A string filename to check
- * @param version One of the NexusHDF5Descriptor::Version enumerations specifying
- * the required version
- * @return True if the file is considered hierarchical, false otherwise
- */
-bool NexusHDF5Descriptor::isReadable(const std::string &filename, const NexusHDF5Descriptor::Version version) {
-  return getHDFVersion(filename) == version;
-}
-
-NexusHDF5Descriptor::Version NexusHDF5Descriptor::getHDFVersion(const std::string &filename) {
-  FILE *fd = fopen(filename.c_str(), "rb");
-  if (!fd) {
-    throw std::invalid_argument("HierarchicalFileDescriptor::isHierarchical - Unable to open file '" + filename + "'");
-  }
-  const NexusHDF5Descriptor::Version result = HDFversion(fd); // use anonymous helper
-  fclose(fd);
-  return result;
-}
-
 NexusHDF5Descriptor::NexusHDF5Descriptor(std::string filename)
     : m_filename(std::move(filename)), m_extension(), m_firstEntryNameType(), m_allEntries(initAllEntries()) {}
 
@@ -156,7 +91,7 @@ std::map<std::string, std::set<std::string>> NexusHDF5Descriptor::initAllEntries
   try {
     fileID = H5::H5File(m_filename, H5F_ACC_RDONLY, H5::FileCreatPropList::DEFAULT, access_plist);
   } catch (const H5::FileIException &) {
-    return allEntries;
+    throw std::invalid_argument("ERROR: Kernel::NexusHDF5Descriptor couldn't open hdf5 file " + m_filename + "\n");
   }
 
   H5::Group groupID = fileID.openGroup("/");

Framework/Kernel/test/NexusHDF5DescriptorTest.h

@@ -34,31 +34,6 @@ std::string getFullPath(const std::string &filename) {
 class NexusHDF5DescriptorTest : public CxxTest::TestSuite {
 
 public:
-  NexusHDF5DescriptorTest() {
-    using Mantid::Kernel::ConfigService;
-    auto dataPaths = ConfigService::Instance().getDataSearchDirs();
-    for (auto &dataPath : dataPaths) {
-      const auto hdf5Path = std::filesystem::path(dataPath) / "CNCS_7860_event.nxs";
-      if (std::filesystem::exists(hdf5Path))
-        m_testHDF5Path = hdf5Path.string();
-
-      const auto hdf4Path = std::filesystem::path(dataPath) / "argus0026287.nxs";
-      if (std::filesystem::exists(hdf4Path))
-        m_testHDF4Path = hdf4Path.string();
-
-      const auto nonhdf5Path = std::filesystem::path(dataPath) / "CSP79590.raw";
-      if (std::filesystem::exists(nonhdf5Path))
-        m_testNonHDFPath = nonhdf5Path.string();
-
-      if (!m_testHDF5Path.empty() && !m_testHDF4Path.empty() && !m_testNonHDFPath.empty())
-        break;
-    }
-    if (m_testHDF5Path.empty() || m_testHDF4Path.empty() || m_testNonHDFPath.empty()) {
-      throw std::runtime_error("Unable to find test files for FileDescriptorTest. "
-                               "The AutoTestData directory needs to be in the search path");
-    }
-  }
-
   // test get functions getFilename and getAllEntries
   void test_nexus_hdf5_descriptor_get() {
 
@@ -118,36 +93,4 @@ class NexusHDF5DescriptorTest : public CxxTest::TestSuite {
     TS_ASSERT(nexusHDF5Descriptor.hasRootAttr("file_name"));
     TS_ASSERT(!nexusHDF5Descriptor.hasRootAttr("not_attr"));
   }
-
-  //=================================== Static isReadable methods
-  //======================================
-  void test_isReadable_Returns_False_For_Non_HDF_Filename() {
-    TS_ASSERT(!NexusHDF5Descriptor::isReadable(m_testNonHDFPath, NexusHDF5Descriptor::Version4));
-    TS_ASSERT(!NexusHDF5Descriptor::isReadable(m_testNonHDFPath, NexusHDF5Descriptor::Version5));
-  }
-
-  void test_isReadable_With_Version4_Returns_True_Only_For_HDF4() {
-    TS_ASSERT(NexusHDF5Descriptor::isReadable(m_testHDF4Path, NexusHDF5Descriptor::Version4));
-    TS_ASSERT(!NexusHDF5Descriptor::isReadable(m_testHDF5Path, NexusHDF5Descriptor::Version4));
-  }
-
-  void test_isReadable_With_Version5_Returns_True_Only_For_HDF4() {
-    TS_ASSERT(NexusHDF5Descriptor::isReadable(m_testHDF5Path, NexusHDF5Descriptor::Version5));
-    TS_ASSERT(!NexusHDF5Descriptor::isReadable(m_testHDF4Path, NexusHDF5Descriptor::Version5));
-  }
-
-  void test_getHDFVersion() {
-    TS_ASSERT_EQUALS(NexusHDF5Descriptor::Version5, NexusHDF5Descriptor::getHDFVersion(m_testHDF5Path));
-    TS_ASSERT_EQUALS(NexusHDF5Descriptor::Version4, NexusHDF5Descriptor::getHDFVersion(m_testHDF4Path));
-    TS_ASSERT_EQUALS(NexusHDF5Descriptor::None, NexusHDF5Descriptor::getHDFVersion(m_testNonHDFPath));
-  }
-
-  void test_isReadable_Throws_With_Invalid_Filename() {
-    TS_ASSERT_THROWS(NexusHDF5Descriptor::isReadable(""), const std::invalid_argument &);
-  }
-
-private:
-  std::string m_testHDF5Path;
-  std::string m_testHDF4Path;
-  std::string m_testNonHDFPath;
 };

scripts/test/corelli/calibration/test_database.py

@@ -433,12 +433,12 @@ def set_daystamp(input_workspace: str, daystamp: int):
             assert list(load_calibration_set(workspace, database_path)) == [None, None]
 
             set_daystamp(workspace, 20200601)
-            with self.assertRaises(RuntimeError) as ar:
+            with self.assertRaises(ValueError) as ar:
                 load_calibration_set(workspace, database_path)  # should pick calibration 20200601
             self.assertEqual("20200601" in str(ar.exception), True)
 
             set_daystamp(workspace, 20201201)
-            with self.assertRaises(RuntimeError) as ar:
+            with self.assertRaises(ValueError) as ar:
                 load_calibration_set(workspace, database_path)
             self.assertEqual("calibration_corelli_20200601.nxs.h5" in str(ar.exception), True)
 
@@ -448,22 +448,22 @@ def set_daystamp(input_workspace: str, daystamp: int):
             assert list(load_calibration_set(workspace, database_path)) == [None, None]
 
             set_daystamp(workspace, "20200401")
-            with self.assertRaises(RuntimeError) as ar:
+            with self.assertRaises(ValueError) as ar:
                 load_calibration_set(workspace, database_path)
             self.assertEqual("calibration_corelli_20200401.nxs.h5" in str(ar.exception), True)
 
             set_daystamp(workspace, "20200601")
-            with self.assertRaises(RuntimeError) as ar:
+            with self.assertRaises(ValueError) as ar:
                 load_calibration_set(workspace, database_path)
             self.assertEqual("calibration_corelli_20200401.nxs.h5" in str(ar.exception), True)
 
             set_daystamp(workspace, "20200801")
-            with self.assertRaises(RuntimeError) as ar:
+            with self.assertRaises(ValueError) as ar:
                 load_calibration_set(workspace, database_path)
             self.assertEqual("calibration_corelli_20200801.nxs.h5" in str(ar.exception), True)
 
             set_daystamp(workspace, "20201201")
-            with self.assertRaises(RuntimeError) as ar:
+            with self.assertRaises(ValueError) as ar:
                 load_calibration_set(workspace, database_path)
             self.assertEqual("calibration_corelli_20200801.nxs.h5" in str(ar.exception), True)
         workspace.delete()