ROOT 6.22, broken operator[] in python for specific Gaudi class

[hassec]

Apologies for making this an annoying bug report :cry:,
but I unfortunately, so far, wasn't able to create a small self-contained reproducer that triggers the bug.

But with LCG_99 and the following snippet I can at least show the problem.

import cppyy
cppyy.cppdef(r"""
using vec = SmartRefVector<DataObject>;
using ref = SmartRef<DataObject>;
        """)

vec = cppyy.gbl.vec()
ref = cppyy.gbl.ref()
vec.push_back(ref)
vec.size()
print(vec[0])
print(vec.at(0))

Running:

source /cvmfs/sft.cern.ch/lcg/views/LCG_99/x86_64-centos7-gcc10-opt/setup.sh
python test.py

will print:

<cppyy.gbl.SmartRefVector<DataObject> object at 0x83e0f00>
<cppyy.gbl.SmartRef<DataObject> object at 0xcf6b7c0>

If you try the same on an older ROOT version e.g. with LCG_97a (source /cvmfs/sft.cern.ch/lcg/views/LCG_97a/x86_64-centos7-gcc9-opt/setup.sh), this will print:

<ROOT.SmartRef<DataObject> object at 0xaac7ac0>
<ROOT.SmartRef<DataObject> object at 0xaac7ac0>

So in ROOT 6.22 the operator[] in python somehow doesn't work correctly for this class. Now as I said, I wish I could have provided an easier reproducer, and I'll have another go at it tomorrow, but for now this is the best I could come up with. The SmartRefVector is part of the Gaudi project and can be found here.
I've tried creating a simplified version of the SmartRefVector, but that didn't exhibit the same problem so it's apparently not as simple as blaming it on the inheritance on std::vector which of course isn't ideal... :see_no_evil:

[hassec]

Couldn't quite let it rest for the day yet :D Here is a smaller reproducer that shows the problem

import cppyy

cppyy.cppdef(r"""
template <typename TYPE>
struct vec : public std::vector<TYPE> {
  vec<TYPE>& operator()(int ){return *this;}
};

using my_tmp = vec<double>;
""")
b = cppyy.gbl.my_tmp()
b.emplace_back()
print(b.__getitem__.func_doc)
print(b[0])

which prints:

vec<double>& vec<double>::operator()(int)
<cppyy.gbl.vec<double> object at 0x6f97bb0>

So it seems that defining a custom call operator somehow results in this bug since that operator() is bound to __getitem__. Seems to be independent of the signature, e.g. void operator()(){} will also get bound to __getitem__.

fyi, when I try this locally with cppyy 1.9.2 I get the following output:

vec<double>& vec<double>::operator()(int)
{ 0.0000000 }

Edit, now that I kind of know what to look for I also found this issue on cppyy which seems related

[etejedor]

Thanks for the reproducer!

Indeed the cppyy issue you mentioned above explains the reason of this behaviour. Note that what you get from b[0] in cppyy 1.9.2 is still a vector as in PyROOT, it's just that it's printed differently.

The cppyy issue also explains that there is a fix for the case the call operator returns by value. I could update PyROOT's cppyy with that fix, but I understand this wouldn't help you in your case?

[hassec]

Note that what you get from b[0] in cppyy 1.9.2 is still a vector as in PyROOT, it's just that it's printed differently.

Thanks, I actually did miss that :see_no_evil:

But you are right, our use case wouldn't be fixed because this specific piece of code returns a reference.
Not sure what the best way forward would be. IMHO, breaking operator[ ] in favor of operator () isn't great, especially since this happens without a warning of any kind :/

[etejedor]

One option is to add a pythonization to your vector class to use the __getitem__ of std::vector.

Something like this:

import cppyy

cppyy.cppdef(r"""
template <typename TYPE>
struct vec : public std::vector<TYPE> {
  vec<TYPE>& operator()(int ){return *this;}
};

using my_tmp = vec<double>;
""")

def pythonizor(klass, name):
    if name.startswith('vec<double>'):
        klass.__getitem__ = cppyy.gbl.std.vector('double').__getitem__

cppyy.py.add_pythonization(pythonizor)

b = cppyy.gbl.my_tmp()
b.emplace_back()
print(b[0])

It would need to be generalized to that the template parameter is parsed (here I just show the example for double).

Note that the pythonizations are injected lazily: only when your class is actually used for the first time, the injection will happen. More info here: https://cppyy.readthedocs.io/en/latest/pythonizations.html#python-callbacks

[hassec]

Thanks, that seems like a good workaround for the reproducer. But, if I understand correctly, that would require that I always include that in a python script before using that class, which seems hard. There isn't one global entry point where cppyy is loaded where I could just add this to protect every user that would possibly interact with the problematic class.

Are you aware of any examples of how to use the C++ callbacks that are mentioned on the same page?
That could work as a more robust solution I think.

[etejedor]

Indeed, you'd need to register the pythonizor before using your class.

Regarding the C++ callbacks, I never used them myself, but in the cppyy test suite: https://bitbucket.org/wlav/cppyy/src/master/test/

there is https://bitbucket.org/wlav/cppyy/src/master/test/pythonizables.h https://bitbucket.org/wlav/cppyy/src/master/test/pythonizables.cxx

(look for explicit_pythonize)

That should work, yes, it's just a bit more verbose because you have to use the Python C API.

[hassec]

Thanks a lot for your help, it's much appreciated :) I think that solves it for me, I now have multiple ways around the issue.

IMHO it would have been nicer if the default would be the exact opposite workflow. That is operator[ ] isn't broken and if really needed for a specific class once could use special pythonization to map __call__ to __getitem__...

Anyway I'll leave it to you if you want to consider this solved or leave it open. Thanks again for the quick help! :+1:

[etejedor]

Yes, I agree this is at least debatable. I have decided to stick for now to what cppyy currently provides and add the fix for the by-value one-arg case: https://github.com/root-project/root/pull/7209

thus keeping the current behaviour otherwise (as in your reproducer), since the current implementation covers other use cases that are explained in the cppyy ticket.

Thanks again for the report!