root icon indicating copy to clipboard operation
root copied to clipboard

RResulPtr could better convey/share ownership of pointee

Open vepadulano opened this issue 1 year ago • 0 comments

Explain what you would like to see improved and how.

This forum post https://root-forum.cern.ch/t/too-many-operators-for-the-ttree-project-conditions/58009/5 triggered a discussion that probably RResultPtr could better behave with respect to ownership of its pointee. For example, this function

TH1D *DrawStuff(const char *columnname) {
   auto file = TFile::Open("tutorials/hsimple.root", "READ");
   auto ntuple = file->Get<TTree>("ntuple");
   ROOT::RDataFrame df(*ntuple);
   auto ht = df.Histo1D(*h, columname);
   return ht.GetPtr();
}

Will return a dangling pointer since the RResultPtr going out of scope will destroy the pointee. The forum user presents a pattern such as

    THStack* histStack = new THStack("histStack", "Bs_MCORR");
    for(int i=0; i<sampNumb; i++) {
        TFile* file = new TFile(rootList[i]);
        TTree* tree = (TTree*)file->Get("DecayTree");
        ROOT::RDataFrame df(*tree);
        auto ht = df.Filter(conds[i].Data()).Histo1D(h, "Bs_MCORR");
        histStack->Add(ht.GetPtr(), , "HIST && E1 && EX0");
   }
   histStack->Draw();

Which also doesn't work with the current API. Admittedly, this example would not work in general since THStack does not take ownership of the passed histograms and assumes they will be kept alive by ROOT global objects. In general, for this kind of use case we could convey the message to users they can store the histograms in a vector of shared pointers which is the clearest and most standard data structure to represent the situation correctly. So we need:

  • std::shared_ptr<T> RResultPtr<T>::GetSharedPtr()
  • More documentation on the RDataFrame user guide about these use cases
  • A tutorial starting from the forum post reproducer once we have the API

ROOT version

Any

Installation method

Any

Operating system

Any

Additional context

No response

vepadulano avatar Feb 19 '24 22:02 vepadulano