Skip to content

Conversation

@hahnjo
Copy link
Member

@hahnjo hahnjo commented Nov 6, 2025

This needs one indirection to construct the std::array of indices with the right (number of) elements.

This needs one indirection to construct the std::array of indices
with the right (number of) elements.
@hahnjo hahnjo self-assigned this Nov 6, 2025
@hahnjo hahnjo added the in:Hist label Nov 6, 2025
@hahnjo
Copy link
Member Author

hahnjo commented Nov 6, 2025

After discussion with Jakob, we may decide that we don't want RHistEngine::SetBinContent in the public interface. We don't want it in RHist because the semantics are unclear together with global histogram statistics. That's not a problem for RHistEngine, but it is still questionable if users should call it. On the other hand, RHistEngine::SetBinContent is useful for testing, for example the conversion to TH*, but that we could also do with Internal functions that we befriend...

@hahnjo hahnjo marked this pull request as draft November 6, 2025 13:11
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Test Results

    22 files      22 suites   3d 19h 6m 58s ⏱️
 3 707 tests  3 707 ✅ 0 💤 0 ❌
79 579 runs  79 579 ✅ 0 💤 0 ❌

Results for commit f7271a8.

@hahnjo
Copy link
Member Author

hahnjo commented Nov 12, 2025

We decided to not add this method to the public interface because it has unclear semantics for a number of cases, in particular related to global histogram statistics. While it might be fine to allow it in RHistEngine, it would also be confusing to then not have it in RHist. #20392 removes the two mentions we had so far.

@hahnjo hahnjo closed this Nov 12, 2025
@hahnjo hahnjo deleted the hist-set branch November 12, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant