Skip to content

Allow I/O of interpreted class if the underlying data format allows#22234

Merged
vepadulano merged 1 commit into
root-project:masterfrom
vepadulano:rdf-awkward-snapshot
May 15, 2026
Merged

Allow I/O of interpreted class if the underlying data format allows#22234
vepadulano merged 1 commit into
root-project:masterfrom
vepadulano:rdf-awkward-snapshot

Conversation

@vepadulano
Copy link
Copy Markdown
Member

@vepadulano vepadulano commented May 11, 2026

RDataFrame needs the type_info of input column types to create the column readers. This is searched first in a map of known simple types, then through TClass. Sometimes TClass might not know about a type_info even though it's available to the interpreter. This is the case for example of runtime-generated classes that are declared to the interpreter but do not have a dictionary, in which case tclass->IsLoaded() will return false and the TClass will not expose the type_info but the interpreter knows about it.

This commit proposes to extend the search in RDataFrame to include also a call to the interpreter in case the RTTI cannot be found through TClass.

This removes a long-standing safeguard introduced in RDataFrame that would prevent users from trying to Snapshot a column if the type had no dictionary. But both TTree and RNTuple support this to different degrees. By removing the safeguard and querying the interpreter for the RTTI RDataFrame delegates responsability to the underlying I/O technology.

As a side effect, this also helps in the integration with awkward arrays, where a C++ type is generated per different awkward array layout at runtime in the Python application. The awkward data source is still responsible to communicate correctly to TTree or RNTuple what they should store to disk when a Snapshot is called.

FYI @ianna

Fixes #22233

Comment thread tree/dataframe/src/RDFUtils.cxx Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 11, 2026

Test Results

    22 files      22 suites   3d 11h 20m 3s ⏱️
 3 854 tests  3 853 ✅ 0 💤 1 ❌
77 017 runs  77 016 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit e7b6af3.

♻️ This comment has been updated with latest results.

@vepadulano vepadulano force-pushed the rdf-awkward-snapshot branch 2 times, most recently from ed725fc to fb44bbe Compare May 12, 2026 14:06
@vepadulano vepadulano requested a review from bellenot as a code owner May 12, 2026 14:06
@vepadulano vepadulano changed the title Search more thoroughly for the RTTI of interpreted types Allow I/O of interpreted class if the underlying data format allows May 12, 2026
@vepadulano vepadulano requested review from jblomer and pcanal May 12, 2026 14:06
vepadulano added a commit to vepadulano/awkward that referenced this pull request May 13, 2026
Change the implementation of the generated RDataSource class to use the newer signature that returns one column reader. This, together with the changes at root-project/root#22234, allow to restore the behaviour that awkward relied upon to store arrays to disk via RDataFrame. Irrespective of these changes, the current in-memory layout of the awkward array is not fit for storage, leading to writing silently garbage data to disk.

Fixes scikit-hep#3885
vepadulano added a commit to vepadulano/awkward that referenced this pull request May 13, 2026
Change the implementation of the generated RDataSource class to use the newer signature that returns one column reader. This, together with the changes at root-project/root#22234, allow to restore the behaviour that awkward relied upon to store arrays to disk via RDataFrame. Irrespective of these changes, the current in-memory layout of the awkward RecordArray is not fit for storage, leading to writing silently garbage data to disk.

Fixes scikit-hep#3885
Comment thread tree/dataframe/src/RDFUtils.cxx
Comment thread tree/dataframe/test/dataframe_snapshot_interpreted_class_read.cxx Outdated
Copy link
Copy Markdown
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

LGTM in general, but I agree with @pcanal's comments.

And if you could move the cleanup to the read test, we could save a source file and a test.

Comment thread tree/dataframe/test/dataframe_snapshot_interpreted_class_cleanup.cxx Outdated
Comment thread tree/dataframe/test/dataframe_snapshot_interpreted_class_read.cxx Outdated
Comment thread tree/dataframe/test/dataframe_snapshot_interpreted_class_read.cxx
RDataFrame needs the type_info of input column types to create the column readers. This is searched first in a map of known simple types, then through TClass. Sometimes TClass might not know about a type_info even though it's available to the interpreter. This is the case for example of runtime-generated classes that are declared to the interpreter but do not have a dictionary, in which case `tclass->IsLoaded()` will return false and the TClass will not expose the type_info but the interpreter knows about it.

This commit proposes to extend the search in RDataFrame to include also a call to the interpreter in case the RTTI cannot be found through TClass.

This removes a long-standing safeguard introduced in RDataFrame that would prevent users from trying to Snapshot a column if the type had no dictionary. But both TTree and RNTuple support this to different degrees. By removing the safeguard and searching querying the interpreter for the RTTI RDataFrame delegates responsability to the underlying I/O technology.

As a side effect, this also helps in the integration with awkward arrays, where a C++ type is generated per different awkward array layout at runtime in the Python application. The awkward data source is still responsible to communicate correctly to TTree or RNTuple what they should store to disk when a Snapshot is called.
@vepadulano vepadulano force-pushed the rdf-awkward-snapshot branch from fb44bbe to e7b6af3 Compare May 15, 2026 08:53
@vepadulano vepadulano merged commit 33b7a3c into root-project:master May 15, 2026
29 of 31 checks passed
@vepadulano
Copy link
Copy Markdown
Member Author

/backport to 6.40

@root-project-bot
Copy link
Copy Markdown

Preparing to backport PR #22234 to branch 6.40 requested by vepadulano

@root-project-bot
Copy link
Copy Markdown

This PR has been backported to branch 6.40: #22303

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.

Regression in previously implicit behaviour in RDataFrame Snapshot

5 participants