Skip to content

[ntuple] Change signature of RRealField::SetQuantized and deprecate old one#22084

Merged
silverweed merged 1 commit intoroot-project:masterfrom
silverweed:rfield_setquantized
Apr 30, 2026
Merged

[ntuple] Change signature of RRealField::SetQuantized and deprecate old one#22084
silverweed merged 1 commit intoroot-project:masterfrom
silverweed:rfield_setquantized

Conversation

@silverweed
Copy link
Copy Markdown
Contributor

@silverweed silverweed commented Apr 28, 2026

Instead of

   SetQuantized(T min, T max, size_t nBits)

use

   SetQuantized(size_t nBits, pair<T, T> valueRange)

Rationale:

  • keep nBits in first position to be more consistent with SetTruncated
  • prevent mistakes of argument wrong ordering where nBits is passed first instead of min.

Instead of
    SetQuantized(T min, T max, size_t nBits)
use
    SetQuantized(size_t nBits, pair<T, T> valueRange)

Rationale:
- keep nBits in first position to be more consistent with SetTruncated
- prevent mistakes of argument wrong ordering where nBits is passed
  first instead of min.
@silverweed silverweed requested review from enirolf and hahnjo April 28, 2026 09:18
@silverweed silverweed self-assigned this Apr 28, 2026
@silverweed silverweed requested a review from jblomer as a code owner April 28, 2026 09:18
Comment thread tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx
@github-actions
Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 12h 14m 36s ⏱️
 3 848 tests  3 848 ✅ 0 💤 0 ❌
76 908 runs  76 908 ✅ 0 💤 0 ❌

Results for commit fd935a3.

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.

Needs backporting to 6.40 if to be deprecated in 6.42.

@hahnjo
Copy link
Copy Markdown
Member

hahnjo commented Apr 29, 2026

If we deprecate in 6.40 and remove "already" in 6.42 (which needs synchronization with #22023), users will have little time to migrate. Maybe that's fine because 6.40 will be an LTS and probably few users call SetQuantized, apart from the RNTuple validation suite...

@silverweed
Copy link
Copy Markdown
Contributor Author

@hahnjo I'm pretty sure the quantized floats are advanced enough to be currently almost unused and the change is purely mechanical, so I think we can afford it. But I'm also fine deprecating it in 6.40 and removing it in 6.44.

@silverweed silverweed added this to the 6.40.00 milestone Apr 29, 2026
Copy link
Copy Markdown
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.

Fair enough. Still need to pay attention after #22023 is now merged, in master it must be immediately removed.

@silverweed silverweed merged commit 900fbdf into root-project:master Apr 30, 2026
33 checks passed
@silverweed
Copy link
Copy Markdown
Contributor Author

/backport to 6.40

@root-project-bot
Copy link
Copy Markdown

Preparing to backport PR #22084 to branch 6.40 requested by silverweed

@root-project-bot
Copy link
Copy Markdown

This PR has been backported to branch 6.40: #22107

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.

4 participants