[df] Improve warning message when lazy Snapshot is not triggered

The current message is not clear enough and may lead to confusing situations such as the one reported at https://root-forum.cern.ch/t/some-problem-with-lazy-snapshots-in-rdataframe/46740
root-project · Jan 12, 2025 · a40bf24 · a40bf24
1 parent e10a74e
commit a40bf24
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 6 deletions.
diff --git a/tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx b/tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx
@@ -1518,8 +1518,18 @@ public:
    SnapshotHelper(SnapshotHelper &&) = default;
    ~SnapshotHelper()
    {
-      if (!fTreeName.empty() /*not moved from*/ && !fOutputFile /* did not run */ && fOptions.fLazy)
-         Warning("Snapshot", "A lazy Snapshot action was booked but never triggered.");
+      if (!fTreeName.empty() /*not moved from*/ && !fOutputFile /* did not run */ && fOptions.fLazy) {
+         const auto fileOpenMode = [&]() {
+            TString checkupdate = fOptions.fMode;
+            checkupdate.ToLower();
+            return checkupdate == "update" ? "updated" : "created";
+         }();
+         Warning("Snapshot",
+                 "A lazy Snapshot action was booked but never triggered. The tree '%s' in output file '%s' was not %s. "
+                 "In case it was desired instead, remember to trigger the Snapshot operation, by storing "
+                 "its result in a variable and for example calling the GetValue() method on it.",
+                 fTreeName.c_str(), fFileName.c_str(), fileOpenMode);
+      }
    }
 
    void InitTask(TTreeReader *r, unsigned int /* slot */)
@@ -1673,8 +1683,18 @@ public:
    ~SnapshotHelperMT()
    {
       if (!fTreeName.empty() /*not moved from*/ && fOptions.fLazy && !fOutputFiles.empty() &&
-          std::all_of(fOutputFiles.begin(), fOutputFiles.end(), [](const auto &f) { return !f; }) /* never run */)
-         Warning("Snapshot", "A lazy Snapshot action was booked but never triggered.");
+          std::all_of(fOutputFiles.begin(), fOutputFiles.end(), [](const auto &f) { return !f; }) /* never run */) {
+         const auto fileOpenMode = [&]() {
+            TString checkupdate = fOptions.fMode;
+            checkupdate.ToLower();
+            return checkupdate == "update" ? "updated" : "created";
+         }();
+         Warning("Snapshot",
+                 "A lazy Snapshot action was booked but never triggered. The tree '%s' in output file '%s' was not %s. "
+                 "In case it was desired instead, remember to trigger the Snapshot operation, by storing "
+                 "its result in a variable and for example calling the GetValue() method on it.",
+                 fTreeName.c_str(), fFileName.c_str(), fileOpenMode);
+      }
    }
 
    void InitTask(TTreeReader *r, unsigned int slot)

diff --git a/tree/dataframe/test/dataframe_snapshot.cxx b/tree/dataframe/test/dataframe_snapshot.cxx
@@ -715,7 +715,11 @@ void BookLazySnapshot()
 
 TEST(RDFSnapshotMore, LazyNotTriggered)
 {
-   ROOT_EXPECT_WARNING(BookLazySnapshot(), "Snapshot", "A lazy Snapshot action was booked but never triggered.");
+   ROOT_EXPECT_WARNING(BookLazySnapshot(), "Snapshot",
+                       "A lazy Snapshot action was booked but never triggered. The tree 't' in output file "
+                       "'lazysnapshotnottriggered_shouldnotbecreated.root' was not created. "
+                       "In case it was desired instead, remember to trigger the Snapshot operation, by "
+                       "storing its result in a variable and for example calling the GetValue() method on it.");
 }
 
 RResultPtr<RInterface<RLoopManager, void>> ReturnLazySnapshot(const char *fname)
@@ -1286,7 +1290,11 @@ TEST(RDFSnapshotMore, JittedSnapshotAndAliasedColumns)
 TEST(RDFSnapshotMore, LazyNotTriggeredMT)
 {
    ROOT::EnableImplicitMT(4);
-   ROOT_EXPECT_WARNING(BookLazySnapshot(), "Snapshot", "A lazy Snapshot action was booked but never triggered.");
+   ROOT_EXPECT_WARNING(BookLazySnapshot(), "Snapshot",
+                       "A lazy Snapshot action was booked but never triggered. The tree 't' in output file "
+                       "'lazysnapshotnottriggered_shouldnotbecreated.root' was not created. "
+                       "In case it was desired instead, remember to trigger the Snapshot operation, by "
+                       "storing its result in a variable and for example calling the GetValue() method on it.");
    ROOT::DisableImplicitMT();
 }