Skip to content

Commit cfc5ab2

Browse files
vepadulanodpiparo
authored andcommitted
Try alternative names in REntry._GetPtr
Fixes the case of instantiating a C++ function template via a Python TemplateProxy when a template argument passed by string (and thus lazily evaluated) is of a type that has alternative names available in the type system. Previously, only the user-provided type name was tried to instantiate the function template `REntry::GetPtr` from the RNTuple pythonization. Now, the pythonization first tries to instantiate the function template via the user-provided string and, failing that, it falls back to retrying the instantiation using all the available alternative names for the name of the field type. A new API in TClassTable is made available for this purpose. The list of available alternative names for an input type name can now be queried via `TClassTable::GetClassAlternativeNames`. One advantage of this approach is that it does not require any change in the RNTuple library and in cppyy. Also, it does not involve any change in behaviour in core, the new API is completely detached from any pre-existing code paths. One disadvantage is that in order to actually trigger the right function template instantiation there needs to be some manual work, currently done in the Pythonization of `REntry::GetPtr`. In general, if a user were to write a C++ template function that needs to take in a template argument type of the same kind of AtlasLikeDataVector, they would need to try the first template instantiation, see it failing, then retry again after querying the list of alternative class names. This workflow is exemplified with a test. Making this approach fully generic would require changes in cppyy which on the surface look quite invasive. For example if a function has multiple template arguments and all of them have multiple alternative class names, one would probably need to trigger all the combinations of template instantiations to look for the right one. cherry-picked from f1fdd79 modulo the changes made in roottest, which is not part of the root repository in the v6-36-00-patches branch.
1 parent 8308a3d commit cfc5ab2

File tree

3 files changed

+76
-2
lines changed

3 files changed

+76
-2
lines changed

bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rntuple.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,46 @@
1515
def _REntry_GetPtr(self, key):
1616
raise RuntimeError("GetPtr is not supported in Python, use indexing")
1717

18+
def _try_getptr(entry, fieldType, key):
19+
rentry_getptr_typeerrors = []
20+
try:
21+
return entry._GetPtr[fieldType](key)
22+
except TypeError as e:
23+
# The input field type name might not be found when trying to instantiate
24+
# the template via the Python bindings. At the moment, there is one
25+
# notable case when this happens. If the type name corresponds to a class
26+
# that has alternative names registered via its dictionary, the name
27+
# passed by the user may not be the fully qualified name seen by the
28+
# compiler. We tap into the knowledge of TClassTable to look for all the
29+
# alternative names, and try if there is a correspondence with one of those.
30+
# Note: clearly a prerequisite for the following section to work is that
31+
# the dictionary of the class was loaded and its alternative names were
32+
# registered. This currently happens at loading time of the RNTupleModel
33+
# which loads the classes found in the schema and also during the first
34+
# attempt at calling _GetPtr with the user-provided class name (in
35+
# particular when it tries to instantiate the function template)
36+
rentry_getptr_typeerrors.append(e)
37+
import ROOT
38+
alt_field_type_names = ROOT.TClassTable.GetClassAlternativeNames(fieldType)
39+
for alt_field_type_name in alt_field_type_names:
40+
try:
41+
# Need to convert std::string to Python string
42+
return entry._GetPtr[str(alt_field_type_name)](key)
43+
except TypeError as alt_e:
44+
rentry_getptr_typeerrors.append(alt_e)
45+
46+
err_msg = f"Failed to retrieve entry value for field type name '{fieldType}'. Full stack trace follows:\n"
47+
for ex in rentry_getptr_typeerrors:
48+
err_msg += str(ex) + "\n"
49+
50+
raise TypeError(err_msg)
1851

1952
def _REntry_CallGetPtr(self, key):
2053
# key can be either a RFieldToken already or a string. In the latter case, get a token to use it twice.
2154
if not hasattr(type(key), "__cpp_name__") or type(key).__cpp_name__ != "ROOT::RFieldToken":
2255
key = self.GetToken(key)
2356
fieldType = self.GetTypeName(key)
24-
return self._GetPtr[fieldType](key)
57+
return _try_getptr(self, fieldType, key)
2558

2659

2760
def _REntry_getitem(self, key):

core/cont/inc/TClassTable.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "TObject.h"
2626
#include <string>
2727
#include <atomic>
28+
#include <vector>
2829

2930
class TProtoClass;
3031

@@ -80,6 +81,7 @@ friend class TROOT;
8081
Int_t pragmabits);
8182
static void Add(TProtoClass *protoClass);
8283
static ROOT::TClassAlt *AddAlternate(const char *normname, const char *alternate);
84+
static std::vector<std::string> GetClassAlternativeNames(const char *cname);
8385
static char *At(UInt_t index);
8486
int Classes();
8587
static Bool_t Check(const char *cname, std::string &normname);

core/cont/src/TClassTable.cxx

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ All functions in TClassTable are thread-safe.
4141
#include <cstdlib>
4242
#include <string>
4343
#include <mutex>
44+
#include <unordered_map>
4445

4546
using namespace ROOT;
4647

@@ -258,6 +259,12 @@ namespace ROOT {
258259
}
259260
}
260261

262+
std::unordered_map<ROOT::TClassRec *, std::vector<ROOT::TClassAlt *>> &GetClassRecToAltMap()
263+
{
264+
static std::unordered_map<ROOT::TClassRec *, std::vector<ROOT::TClassAlt *>> classRecToAltMap;
265+
return classRecToAltMap;
266+
}
267+
261268
////////////////////////////////////////////////////////////////////////////////
262269
/// TClassTable is a singleton (i.e. only one can exist per application).
263270

@@ -270,7 +277,7 @@ TClassTable::TClassTable()
270277
fgAlternate = new TClassAlt* [fgSize];
271278
fgIdMap = new IdMap_t;
272279
memset(fgTable, 0, fgSize * sizeof(TClassRec*));
273-
memset(fgAlternate, 0, fgSize * sizeof(TClassAlt*));
280+
memset(fgAlternate, 0, fgSize * sizeof(TClassAlt *));
274281
gClassTable = this;
275282

276283
for (auto &&r : GetDelayedAddClass()) {
@@ -528,6 +535,17 @@ ROOT::TClassAlt* TClassTable::AddAlternate(const char *normName, const char *alt
528535
}
529536

530537
fgAlternate[slot] = new TClassAlt(alternate,normName,fgAlternate[slot]);
538+
539+
UInt_t slotNorm = ROOT::ClassTableHash(normName, fgSize);
540+
if (fgTable[slotNorm]) {
541+
auto &classToAlt = GetClassRecToAltMap();
542+
// Let others connect a class record to its class alternative names
543+
if (auto it = classToAlt.find(fgTable[slotNorm]); it == classToAlt.end())
544+
classToAlt[fgTable[slotNorm]] = std::vector<ROOT::TClassAlt *>{fgAlternate[slot]};
545+
else
546+
classToAlt[fgTable[slotNorm]].push_back(fgAlternate[slot]);
547+
}
548+
531549
return fgAlternate[slot];
532550
}
533551

@@ -1030,3 +1048,24 @@ TNamed *ROOT::RegisterClassTemplate(const char *name, const char *file,
10301048
}
10311049
return reg;
10321050
}
1051+
1052+
std::vector<std::string> TClassTable::GetClassAlternativeNames(const char *cname)
1053+
{
1054+
std::lock_guard<std::mutex> lock(GetClassTableMutex());
1055+
1056+
UInt_t slot = ROOT::ClassTableHash(cname, fgSize);
1057+
if (!fgTable[slot])
1058+
return {};
1059+
const auto &classRecToAltMap = GetClassRecToAltMap();
1060+
if (auto it = classRecToAltMap.find(fgTable[slot]); it == classRecToAltMap.end())
1061+
return {};
1062+
1063+
const auto &classAlts = classRecToAltMap.at(fgTable[slot]);
1064+
std::vector<std::string> ret;
1065+
ret.reserve(classAlts.size());
1066+
for (const auto *classAlt : classAlts) {
1067+
ret.push_back(classAlt->fName);
1068+
}
1069+
1070+
return ret;
1071+
}

0 commit comments

Comments
 (0)