Skip to content

Conversation

@afroch
Copy link
Contributor

@afroch afroch commented May 22, 2025

Summary

This pull request introduces the following changes

  • Histogram Refactor: Binning/Normalisation is now executed once during Histogram construction instead of every draw call, making plotting faster and keeping per-instance state consistent.
  • Persistence Layer: Adds save()/load() logic (eventually moved to PlotLineObject) so any histogram can be serialised to JSON/YAML and reconstructed later, including NumPy arrays and styling tuples.
  • ROC Curves: ROC curve objects can now be saved/loaded, similar to Histogram objects.
  • Support for High-Level-Plots: Support for high-level plots with automatic saving/loading of Histogram/ROC objects. Only for Results for now. AuxResults will need bigger changes.
  • Updated Plotting Flow: Introduces a new “histogram plotting order” and propagates it up to the high-level plot helpers.
  • Docs + Examples: New example script shows how to store and reload histograms; corresponding user-guide pages and the changelog are added.
  • Robustness: Fresh unit tests cover the new code

Migration notes

  • Existing plot calls continue to work; however, if user code relied on lazy histogram construction it should now create Histogram objects first and re-use them.
  • Saving/loading is opt-in; nothing is written unless save() is invoked.

Overall, this PR modernises histogram handling by treating them as first-class, serialisable data objects while trimming redundant per-plot computation.

Conformity

@afroch afroch self-assigned this May 22, 2025
@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.34%. Comparing base (831d922) to head (0aedc9e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   98.14%   98.34%   +0.20%     
==========================================
  Files          53       53              
  Lines        5711     5922     +211     
==========================================
+ Hits         5605     5824     +219     
+ Misses        106       98       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@afroch afroch marked this pull request as ready for review May 23, 2025 07:50
@afroch afroch added the No backward compatability This PR will introduce changes that are not backward compatible label May 23, 2025
@afroch afroch requested a review from samvanstroud May 23, 2025 09:00
@afroch afroch changed the title Enable Storage of Histogram Objects Enable Storage of Histogram/ROC Objects May 23, 2025
@dguest
Copy link

dguest commented Jun 10, 2025

Maybe adding noise, but at some point we might want to look at this other effort toward a histogram storage format: scikit-hep/uhi#105. Seems a lot more specific about just storing the data, without the plotting code, but that would be enough to speed our plotting up a lot.

@samvanstroud
Copy link
Contributor

Maybe adding noise, but at some point we might want to look at this other effort toward a histogram storage format: scikit-hep/uhi#105. Seems a lot more specific about just storing the data, without the plotting code, but that would be enough to speed our plotting up a lot.

We need to support other kinds of plots too though :( Not sure if we want to combine a third party solution with an in-house one?

@afroch
Copy link
Contributor Author

afroch commented Jun 10, 2025

I think we can discuss other formats but the issue I fixed in this PR is a general issue. So therefore I would delay adding new third-party solutions to later

@dguest
Copy link

dguest commented Jun 10, 2025

yeah I'm not saying we should delay this. I guess better to make this into an issue right?

See #322.

@samvanstroud samvanstroud merged commit d7ba23e into main Jun 11, 2025
9 checks passed
@samvanstroud samvanstroud deleted the alfroch-update-plotting branch June 11, 2025 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No backward compatability This PR will introduce changes that are not backward compatible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants