Skip to content

feat: add more MC lhe files for pylhe#180

Merged
ariostas merged 9 commits intoscikit-hep:mainfrom
APN-Pucky:more_pylhe_mcs
Jun 23, 2025
Merged

feat: add more MC lhe files for pylhe#180
ariostas merged 9 commits intoscikit-hep:mainfrom
APN-Pucky:more_pylhe_mcs

Conversation

@APN-Pucky
Copy link
Member

@APN-Pucky APN-Pucky commented Jun 15, 2025

We want to add a few lhe{,.gz} files for pylhe tests scikit-hep/pylhe#291:

  • madgraph
  • powheg
  • sherpa
  • pythia
  • herwig (does not export to lhe)
  • whizard

@codecov
Copy link

codecov bot commented Jun 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.38%. Comparing base (5e54716) to head (0a36f0d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #180   +/-   ##
=======================================
  Coverage   72.38%   72.38%           
=======================================
  Files           3        3           
  Lines         134      134           
=======================================
  Hits           97       97           
  Misses         37       37           

☔ 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.

Copy link
Member

@eduardo-rodrigues eduardo-rodrigues left a comment

Choose a reason for hiding this comment

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

Great to see this.

A little question while I approve: are these all files that you produced yourself or should one be adding a README or some other citation in case you copied the files over from somewhere? We had a similar discussion in #67 and ended up adding a .doi file in that particular case.

@APN-Pucky
Copy link
Member Author

APN-Pucky commented Jun 17, 2025

A little question while I approve: are these all files that you produced yourself or should one be adding a README or some other citation in case you copied the files over from somewhere? We had a similar discussion in #67 and ended up adding a .doi file in that particular case.

whizard, sherpa, pythia-8.3.14 and madgraph5-3.5.X I ran myself. The remaining files I took from pythias example folder: https://gitlab.com/Pythia8/releases/-/tree/master/examples?ref_type=heads

@eduardo-rodrigues
Copy link
Member

OK, cool. Then maybe add a my_file_name.readme for those files, stating inside that the file is taken from the link above (latest tag sounds good, since a clear point in time)?

Please merge when happy 👍.

@APN-Pucky
Copy link
Member Author

APN-Pucky commented Jun 17, 2025

I added the readme, but the pre-commit hook removed tailing newlines and trailing spaces so it is not easy to verify files by checksums anymore... Just in case somebody wonders in the future why they are slightly different. (IMO there should be no such pre-commit for the data folder)

@eduardo-rodrigues
Copy link
Member

I added the readme, but the pre-commit hook removed tailing newlines and trailing spaces so it is not easy to verify files by checksums anymore... Just in case somebody wonders in the future why they are slightly different. (IMO there should be no such pre-commit for the data folder)

Ah, I had not seen that. Fair enough one should ignore the data folder ... Or else filter on file types, as in the block https://github.com/scikit-hep/particle/blob/main/.pre-commit-config.yaml#L21.

@APN-Pucky
Copy link
Member Author

Please merge when happy 👍.

Who can merge it? I do not have access to scikit-hep-testdata. Can we get a new release then too, because I want to add the files in pylhe to the tests too.

@eduardo-rodrigues
Copy link
Member

Hey, I'm presently at a conference. Can you email the list to get somebody to merge and release? Else I will try later, but you probably want to proceed. Thanks.

@ariostas
Copy link
Member

I can merge and make a release. Do you want to fix the issue with pre-commit first, or just merge as is?

@APN-Pucky
Copy link
Member Author

Thanks @ariostas. Since it is probably better, I will update the pre-commit and then recopy the files so that the hashes agree again.

@APN-Pucky
Copy link
Member Author

@ariostas I excluded .lhe (and .lhe.gz, which is maybe not strictly needed) from the precommit now and it can be merged, IMO.

@ariostas
Copy link
Member

Sounds good, thanks @APN-Pucky! I'll merge and make a release now.

@ariostas ariostas changed the title Add more MC lhe files for pylhe feat: add more MC lhe files for pylhe Jun 23, 2025
@ariostas ariostas merged commit c896d72 into scikit-hep:main Jun 23, 2025
7 checks passed
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