Skip to content

Commit 387c01b

Browse files
committed
[RDF] Forbid calling VariationsFor on snapshots.
Given that snapshots with variations will be enabled via RSnapshotOptions, the snapshot overload for Vary can be removed, and a static_assert can be used to signal the failure.
1 parent 453a817 commit 387c01b

File tree

3 files changed

+7
-32
lines changed

3 files changed

+7
-32
lines changed

tree/dataframe/inc/ROOT/RDFHelpers.hxx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,10 @@ namespace Experimental {
222222
template <typename T>
223223
RResultMap<T> VariationsFor(RResultPtr<T> resPtr)
224224
{
225+
using SnapshotResult_t = ROOT::RDF::RInterface<ROOT::Detail::RDF::RLoopManager, void>;
226+
static_assert(!std::is_same_v<T, SnapshotResult_t>,
227+
"Snapshot with variations only can be enabled via RSnapshotOptions.");
228+
225229
R__ASSERT(resPtr != nullptr && "Calling VariationsFor on an empty RResultPtr");
226230

227231
// populate parts of the computation graph for which we only have "empty shells", e.g. RJittedActions and
@@ -270,9 +274,6 @@ RResultMap<T> VariationsFor(RResultPtr<T> resPtr)
270274
*resPtr.fLoopManager, std::move(nominalAction), std::move(variedAction));
271275
}
272276

273-
using SnapshotPtr_t = ROOT::RDF::RResultPtr<ROOT::RDF::RInterface<ROOT::Detail::RDF::RLoopManager, void>>;
274-
SnapshotPtr_t VariationsFor(SnapshotPtr_t resPtr);
275-
276277
/// \brief Add ProgressBar to a ROOT::RDF::RNode
277278
/// \param[in] df RDataFrame node at which ProgressBar is called.
278279
///

tree/dataframe/src/RDFHelpers.cxx

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,7 @@ unsigned int ROOT::RDF::RunGraphs(std::vector<RResultHandle> handles)
141141
return uniqueLoops.size();
142142
}
143143

144-
ROOT::RDF::Experimental::SnapshotPtr_t ROOT::RDF::Experimental::VariationsFor(ROOT::RDF::Experimental::SnapshotPtr_t)
145-
{
146-
throw std::logic_error("Varying a Snapshot result is not implemented yet.");
147-
}
148-
149-
namespace ROOT {
150-
namespace RDF {
151-
152-
namespace Experimental {
144+
namespace ROOT::RDF::Experimental {
153145

154146
void ThreadsPerTH3(unsigned int N)
155147
{
@@ -398,6 +390,5 @@ void AddProgressBar(ROOT::RDataFrame dataframe)
398390
auto node = ROOT::RDF::AsRNode(dataframe);
399391
ROOT::RDF::Experimental::AddProgressBar(node);
400392
}
401-
} // namespace Experimental
402-
} // namespace RDF
403-
} // namespace ROOT
393+
394+
} // namespace ROOT::RDF::Experimental

tree/dataframe/test/dataframe_vary.cxx

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,23 +1557,6 @@ TEST_P(RDFVary, VaryTake)
15571557
EXPECT_EQ(sorted(rs["x:1"]), std::vector<int>({1, 2, 3}));
15581558
}
15591559

1560-
TEST_P(RDFVary, VarySnapshot)
1561-
{
1562-
const auto fname = "dummy.root";
1563-
auto h = ROOT::RDataFrame(10)
1564-
.Define("x", [](ULong64_t e) { return int(e); }, {"rdfentry_"})
1565-
.Vary(
1566-
"x", [](int x) { return ROOT::RVecI{x - 1, x + 1}; }, {"x"}, 2)
1567-
.Snapshot("t", fname, {"x"});
1568-
EXPECT_THROW(
1569-
try { VariationsFor(h); } catch (const std::logic_error &err) {
1570-
const auto msg = "Varying a Snapshot result is not implemented yet.";
1571-
EXPECT_STREQ(err.what(), msg);
1572-
throw;
1573-
},
1574-
std::logic_error);
1575-
}
1576-
15771560
// this is a regression test, we used to read from wrong addresses in this case
15781561
TEST_P(RDFVary, MoreVariedColumnsThanVariations)
15791562
{

0 commit comments

Comments
 (0)