⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
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 15h 11m 1s ⏱️
 3 770 tests  3 719 ✅ 0 💤 51 ❌
74 986 runs  74 934 ✅ 0 💤 52 ❌

For more details on these failures, see this check.

Results for commit 11610e1b.

♻️ 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.

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

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

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.

3 participants