Skip to content

[ntuple] Some changes/fixes to better handle feature flags#22135

Open
silverweed wants to merge 3 commits intoroot-project:masterfrom
silverweed:ntuple_featureflags
Open

[ntuple] Some changes/fixes to better handle feature flags#22135
silverweed wants to merge 3 commits intoroot-project:masterfrom
silverweed:ntuple_featureflags

Conversation

@silverweed
Copy link
Copy Markdown
Contributor

Based on #22132.

Contains some small changes that I could split off #22017 to avoid introducing too much stuff in it.
This PR doesn't introduce any feature flag per se but it makes the code a bit more robust by dropping the assumption that we have no feature flags.

Checklist:

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

@silverweed silverweed requested review from enirolf and hahnjo May 4, 2026 13:44
@silverweed silverweed self-assigned this May 4, 2026
@silverweed silverweed requested a review from jblomer as a code owner May 4, 2026 13:44
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

Test Results

    22 files      22 suites   3d 10h 47m 41s ⏱️
 3 848 tests  3 847 ✅ 0 💤 1 ❌
76 915 runs  76 913 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 4340c9a.

♻️ This comment has been updated with latest results.

@silverweed silverweed force-pushed the ntuple_featureflags branch 2 times, most recently from fe2cdcb to 3a06a94 Compare May 6, 2026 06:50
Copy link
Copy Markdown
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'm not sure what the warning signs in the file list next to RNTupleSerialize.cxx and ntuple_compat.cxx refer to though.

Comment thread tree/ntuple/inc/ROOT/RNTupleUtils.hxx Outdated
Comment thread tree/ntuple/test/ntuple_compat.cxx Outdated
@silverweed
Copy link
Copy Markdown
Contributor Author

I'm not sure what the warning signs in the file list next to RNTupleSerialize.cxx and ntuple_compat.cxx refer to though.

They will disappear as soon as kFeatureFlag_COUNT becomes larger than 0 (i.e. in the PR of the extended column representations in the merger)

@silverweed silverweed requested a review from pcanal May 6, 2026 11:57
@silverweed silverweed force-pushed the ntuple_featureflags branch from 3a06a94 to 2f32fab Compare May 7, 2026 07:49
@silverweed silverweed force-pushed the ntuple_featureflags branch from 2f32fab to fcef84a Compare May 7, 2026 11:28
@silverweed silverweed force-pushed the ntuple_featureflags branch from fcef84a to b955505 Compare May 8, 2026 09:48
@silverweed silverweed added the clean build Ask CI to do non-incremental build on PR label May 8, 2026
@silverweed silverweed closed this May 8, 2026
@silverweed silverweed reopened this May 8, 2026
@silverweed silverweed force-pushed the ntuple_featureflags branch 2 times, most recently from f7003a3 to 5b5ef5a Compare May 8, 2026 12:14
@silverweed silverweed force-pushed the ntuple_featureflags branch from 5b5ef5a to 4340c9a Compare May 8, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR in:RNTuple

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants