Homogenize behavior of TTree and RNTuple reader

Author: tmadlener

include/podio/ROOTReader.h

@@ -195,8 +195,8 @@ class ROOTReader {
                                                   const std::vector<std::string>& collsToRead);
 
   /// Get / read the buffers at index iColl in the passed category information
-  podio::CollectionReadBuffers getCollectionBuffers(CategoryInfo& catInfo, size_t iColl, bool reloadBranches,
-                                                    unsigned int localEntry);
+  std::optional<podio::CollectionReadBuffers> getCollectionBuffers(CategoryInfo& catInfo, size_t iColl,
+                                                                   bool reloadBranches, unsigned int localEntry);
 
   std::unique_ptr<TChain> m_metaChain{nullptr};                 ///< The metadata tree
   std::unordered_map<std::string, CategoryInfo> m_categories{}; ///< All categories

src/RNTupleReader.cc

@@ -185,13 +185,13 @@ std::unique_ptr<ROOTFrameData> RNTupleReader::readEntry(const std::string& categ
     const auto& collType = coll.dataType;
     const auto& bufferFactory = podio::CollectionBufferFactory::instance();
     const auto maybeBuffers = bufferFactory.createBuffers(collType, coll.schemaVersion, coll.isSubset);
-    const auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{});
 
     if (!maybeBuffers) {
-      std::cout << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType
+      std::cerr << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType
                 << " and schema version " << coll.schemaVersion << std::endl;
       return nullptr;
     }
+    const auto& collBuffers = maybeBuffers.value();
 
     if (coll.isSubset) {
       const auto brName = root_utils::subsetBranch(coll.name);

src/ROOTReader.cc

@@ -133,7 +133,11 @@ std::unique_ptr<ROOTFrameData> ROOTReader::readEntry(ROOTReader::CategoryInfo& c
     if (!collsToRead.empty() && std::ranges::find(collsToRead, catInfo.storedClasses[i].name) == collsToRead.end()) {
       continue;
     }
-    buffers.emplace(catInfo.storedClasses[i].name, getCollectionBuffers(catInfo, i, reloadBranches, localEntry));
+    auto collBuffers = getCollectionBuffers(catInfo, i, reloadBranches, localEntry);
+    if (!collBuffers) {
+      return nullptr;
+    }
+    buffers.emplace(catInfo.storedClasses[i].name, collBuffers.value());
   }
 
   auto parameters = readEntryParameters(catInfo, reloadBranches, localEntry);
@@ -142,17 +146,22 @@ std::unique_ptr<ROOTFrameData> ROOTReader::readEntry(ROOTReader::CategoryInfo& c
   return std::make_unique<ROOTFrameData>(std::move(buffers), catInfo.table, std::move(parameters));
 }
 
-podio::CollectionReadBuffers ROOTReader::getCollectionBuffers(ROOTReader::CategoryInfo& catInfo, size_t iColl,
-                                                              bool reloadBranches, unsigned int localEntry) {
+std::optional<podio::CollectionReadBuffers> ROOTReader::getCollectionBuffers(ROOTReader::CategoryInfo& catInfo,
+                                                                             size_t iColl, bool reloadBranches,
+                                                                             unsigned int localEntry) {
   const auto& name = catInfo.storedClasses[iColl].name;
   const auto& [collType, isSubsetColl, schemaVersion, index] = catInfo.storedClasses[iColl].info;
   auto& branches = catInfo.branches[index];
 
   const auto& bufferFactory = podio::CollectionBufferFactory::instance();
   auto maybeBuffers = bufferFactory.createBuffers(collType, schemaVersion, isSubsetColl);
 
-  // TODO: Error handling of empty optional
-  auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{});
+  if (!maybeBuffers) {
+    std::cerr << "WARNING: Buffers couldn't be created for collection " << name << " of type " << collType
+              << " and schema version " << schemaVersion << std::endl;
+    return std::nullopt;
+  }
+  auto collBuffers = maybeBuffers.value();
 
   if (reloadBranches) {
     root_utils::resetBranches(catInfo.chain.get(), branches, name);

tools/src/podio-dump-tool.cpp

@@ -248,26 +248,21 @@ int main(int argc, char* argv[]) {
   // We strip the executable name off directly for parsing
   const auto args = parseArgs({argv + 1, argv + argc});
 
-  try {
-    auto reader = podio::makeReader(args.inputFile);
-    if (!args.dumpEDM.empty()) {
-      return dumpEDMDefinition(reader, args.dumpEDM);
-    }
+  auto reader = podio::makeReader(args.inputFile);
+  if (!args.dumpEDM.empty()) {
+    return dumpEDMDefinition(reader, args.dumpEDM);
+  }
 
-    printGeneralInfo(reader, args.inputFile);
+  printGeneralInfo(reader, args.inputFile);
 
-    for (const auto event : args.events) {
-      try {
-        const auto& frame = reader.readFrame(args.category, event);
-        printFrame(frame, args.category, event, args.detailed);
-      } catch (std::runtime_error& err) {
-        fmt::println(stderr, "{}", err.what());
-        return 1;
-      }
+  for (const auto event : args.events) {
+    try {
+      const auto& frame = reader.readFrame(args.category, event);
+      printFrame(frame, args.category, event, args.detailed);
+    } catch (std::runtime_error& err) {
+      fmt::println(stderr, "{}", err.what());
+      return 1;
     }
-  } catch (const std::bad_function_call&) {
-    fmt::println(stderr, "ERROR: Cannot create the necessary structures to read a collection from file");
-    return 1;
   }
 
   return 0;