Skip to content

Conversation

@silverweed
Copy link
Contributor

This Pull request:

allows RMiniFileWriter to handle RFile and exposes an experimental way to create an RNTupleWriter using RFile.
Currently the semantics are the same as the TDirectory overload: you pass in a reference to RFile and are responsible to keep the file alive until the RNTupleWriter is done.

Main question to answer: is this the semantics we want? It seems to me that it's the most straightforward API in C++, but it requires some extra work in python to avoid use-after-free cases like:

def CreateWriter():
  file = ROOT.RFile.Open(...)
  writer = ROOT.Experimental.RNTupleWriter_Append(..., file)
  return writer  # <- file may be GC'd from here on

This should be solveable by pythonizing Append to hold on to the file's proxy on the python side, but there might be additional complications I'm not considering. I'm open for discussion.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@github-actions
Copy link

github-actions bot commented Jan 16, 2026

Test Results

    22 files      22 suites   3d 17h 8m 47s ⏱️
 3 774 tests  3 725 ✅ 0 💤 49 ❌
75 949 runs  75 900 ✅ 0 💤 49 ❌

For more details on these failures, see this check.

Results for commit ff09606.

♻️ This comment has been updated with latest results.

@silverweed silverweed force-pushed the rfile_rntuple branch 2 times, most recently from 864f938 to 0f1b792 Compare January 19, 2026 09:29
Copy link
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM code-wise, I'll let Jakob approve the interface part (which I guess you already discussed offline)

Copy link
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I think we should have a follow-up PR to rename the internal file structs in the mini file writer, e.g. RImplSimple, RImplTFile, RImplRFile... or something better. It just seems that currently the code overuses the word File.

@silverweed
Copy link
Contributor Author

Added a python test (and a small pythonization of the RFile-based Append)

Just for the time being, to avoid exposing experimental API as a public
method of the RNTupleWriter, use a free friend function in the Experimental
namespace. When we settle on the RFile/RNTuple writing semantics we will
remove this function and replace it with a factory method.
@silverweed
Copy link
Contributor Author

Since the python test and pythonization are blocked by #21030, for this PR I'll only add the C++ part and will do the python side on a separate one.

@silverweed silverweed merged commit 3fa908c into root-project:master Jan 30, 2026
28 of 30 checks passed
@silverweed silverweed deleted the rfile_rntuple branch January 30, 2026 15:08
operator bool() const { return fDirectory; }
};

struct RFileRFile {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class would benefit from a short introduction to its raison d'être as the name itself is not self explanatory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #21100

@silverweed
Copy link
Contributor Author

LGTM.

I think we should have a follow-up PR to rename the internal file structs in the mini file writer, e.g. RImplSimple, RImplTFile, RImplRFile... or something better. It just seems that currently the code overuses the word File.

#21100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants