Skip to content

Conversation

@wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Dec 7, 2025

Briefly, what does this PR introduce?

This adds an explicit import ROOT before the use of the HepMC3 (incl. rootIO) functionality. This appears to be required for ROOT 6.38.00 (https://eicweb.phy.anl.gov/EIC/benchmarks/detector_benchmarks/-/jobs/6911468#L1101).

I'm not entirely happy by my lack of understanding of why this is necessary...

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an explicit import ROOT statement at the beginning of filterHEPMC3.py to fix compatibility issues with ROOT 6.38.00. The HepMC3 rootIO module requires ROOT to be imported before use, which was not previously necessary in older ROOT versions but is now required.

Key Changes

  • Added import ROOT before importing pyHepMC3 modules to ensure proper initialization of ROOT-based I/O functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

LGTM, but we need to check if it works

@wdconinc
Copy link
Contributor Author

wdconinc commented Dec 8, 2025

LGTM, but we need to check if it works

Checked that this fix works inside the ROOT 6.38 container.

@wdconinc
Copy link
Contributor Author

wdconinc commented Dec 8, 2025

LGTM, but we need to check if it works

Checked that this fix works inside the ROOT 6.38 container.

And it works in CI here...

https://eicweb.phy.anl.gov/EIC/benchmarks/detector_benchmarks/-/jobs/6923712

@wdconinc wdconinc merged commit 1853597 into master Dec 8, 2025
3 checks passed
@wdconinc wdconinc deleted the wdconinc-patch-1 branch December 8, 2025 02:56
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.

3 participants