Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ntuple] support rules with source class != dest class #18357

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jblomer
Copy link
Contributor

@jblomer jblomer commented Apr 10, 2025

Including class rename rules.

@jblomer jblomer added this to the 6.36.00 milestone Apr 10, 2025
@jblomer jblomer self-assigned this Apr 10, 2025
Copy link

Test Results

    18 files      18 suites   4d 16h 1m 6s ⏱️
 2 722 tests  2 722 ✅ 0 💤 0 ❌
47 318 runs  47 318 ✅ 0 💤 0 ❌

Results for commit db746d4.

for (auto linkId : fieldDesc.GetLinkIds()) {
const auto &subFieldDesc = desc.GetFieldDescriptor(linkId);
regularSubfields.insert(subFieldDesc.GetFieldName());
}

rules = FindRules(&fieldDesc);

// If the field's type name is not the on-disk name but we found a rule, we know it is valid to read
// on-disk data because we found the rule according to the on-disk (source) type name and version/checsksum.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// on-disk data because we found the rule according to the on-disk (source) type name and version/checsksum.
// on-disk data because we found the rule according to the on-disk (source) type name and version/checksum.

@ferdymercury
Copy link
Contributor

Maybe related: https://its.cern.ch/jira/browse/ROOT-114

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, see Giacomo's comment about a typo.

Note that, if I remember correctly, it is also possible to write entire class rename rules with only sourceClass and targetClass, but no source and target. I believe then all members are subject to automatic evolution, but should be checked. In a first implementation, we could require explicit copying of all members by the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants