[ntuple] Improvements of Python interface and tests #17369
Conversation
Test Results 16 files 16 suites 3d 20h 39m 45s ⏱️ For more details on these failures, see this check. Results for commit baba720. ♻️ This comment has been updated with latest results. |
bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rntuple.py
Outdated
Show resolved
Hide resolved
| and type(maybe_model).__cpp_name__ == "ROOT::Experimental::RNTupleModel" | ||
| ): | ||
| # In Python, the user cannot create REntries directly from a model, so we can safely clone it and avoid destructively passing the user argument. | ||
| maybe_model = maybe_model.Clone() |
There was a problem hiding this comment.
Here we might have a free opportunity for some extra error handling in an else branch letting the user know that only an RNTupleModel is accepted as input.
There was a problem hiding this comment.
RNTupleReader::Open is overloaded, the first argument can also just be a string. In that case, the model is reconstructed from the on-disk header. I don't know if there is a better way to implement overloaded methods in Python...
There was a problem hiding this comment.
I don't know if there is a better way to implement overloaded methods in Python...
The best approximation of it is https://docs.python.org/3/library/functools.html#functools.singledispatch , I don't know if it's worth using it for this particular PR.
There was a problem hiding this comment.
I also don't know how nicely this plays with cppyy. I'll leave the few extra cases as they are for the moment.
| # In Python, the user cannot create REntries directly from a model, so we can safely clone it and avoid destructively passing the user argument. | ||
| model = model.Clone() |
There was a problem hiding this comment.
Maybe we want to have the same check as in _RNTupleWriter_Recreate here?
There was a problem hiding this comment.
We could, but for RNTupleWriter::Append the first model parameter is mandatory. For Recreate, the user can also pass a list of tuples (type, fieldname).
RNTupleReader::LoadEntry() calls REntry::Read(), so a fwd declaration is not enough.
REntries should be created directly from readers or writers.
As discussed around the RNTuple workshop, forbid calling
RNTupleModel::CreateEntry()directly from Python. This allows to clone the passedRNTupleModelwhen creating a reader or writer, and avoid destructively passing the user argument.