Skip to content

[ntuple] add test for ShiftAliasColumn xHeader patch-up#22747

Open
silverweed wants to merge 1 commit into
root-project:masterfrom
silverweed:ntuple_test_shift_alias
Open

[ntuple] add test for ShiftAliasColumn xHeader patch-up#22747
silverweed wants to merge 1 commit into
root-project:masterfrom
silverweed:ntuple_test_shift_alias

Conversation

@silverweed

Copy link
Copy Markdown
Contributor

Testing the change introduced in d32b64c

The situation to test is:

  • have a projected field in the regular header
  • add a column extension for that field in the extended header
  • call ShiftAliasColumns after adding it
  • verify that the column was shifted properly.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 18h 32m 16s ⏱️
 3 872 tests  3 872 ✅ 0 💤 0 ❌
79 695 runs  79 695 ✅ 0 💤 0 ❌

Results for commit 7c61e59.

♻️ This comment has been updated with latest results.

@jblomer jblomer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In principle OK but slightly confusing because the alias columns of the projected field, in a real descriptor, would evolve in lock-step with their origin columns / field.

@silverweed silverweed force-pushed the ntuple_test_shift_alias branch from 60058e6 to e3211a2 Compare July 7, 2026 09:18
@silverweed

Copy link
Copy Markdown
Contributor Author

@jblomer I updated the test to a more realistic scenario

@silverweed silverweed force-pushed the ntuple_test_shift_alias branch from e3211a2 to 7c61e59 Compare July 8, 2026 13:49

@jblomer jblomer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM


// We are about to add a new physical column, so we need to shift the already-existing alias column.
builder.ShiftAliasColumns(1);
builder.AddColumn(RColumnDescriptorBuilder()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps add a comment that a real descriptor would also add the corresponding alias column for pt_proj.

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.

2 participants