scikit-hep / pylhe

Lightweight Python interface to read Les Houches Event (LHE) files
https://pypi.org/project/pylhe/
Apache License 2.0
41 stars 23 forks source link

feat: Add LHE writing functionality #233

Closed APN-Pucky closed 2 months ago

APN-Pucky commented 8 months ago

I decided to add a tolhe() function to every class (since it is neither xml nor a simple string). The format strings can maybe still be improved? read_lhe_init() now returns a LHEInit object combining weights, procinfo and initinfo. For backwards compatibility I added the get/set there so that LHEInit will still behave as previous LHEInit, what is now LHEInitInfo.

The tests can probably still be extended, but for the first draft I just hardcoded them for one file.

Already includes https://github.com/scikit-hep/pylhe/pull/232 and https://github.com/scikit-hep/pylhe/pull/231 Closes: #229

Squash Commit summary:

feat: Add LHE writing functionality

* Add `LHEInitInfo` dict class that contains only the first line of the <init> block.
  Previously this was in `LHEInit`. The new `LHEInit` contains more lhe-init variables
  `procInfo`, `weightgroup` and `LHEVersion` needed for writing LHE files. For
  backwards compatibility `LHEInit` maps to `LHEInitInfo` unless `procInfo`,
  `weightgroup` or `LHEVersion` are requested.
* Implemented `tolhe` methods for `LHEEvent`, `LHEEventInfo`, `LHEParticle`, `LHEInit`, 
  `LHEInitInfo`, and `LHEProcInfo` classes to support converting objects to LHE
  formatted strings.
* Added new functionality to write LHE files to both string and file, with options for
  gzip compression and different weight formats.
codecov[bot] commented 8 months ago

Codecov Report

Attention: Patch coverage is 90.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 91.17%. Comparing base (584c03f) to head (ef0376f). Report is 2 commits behind head on main.

Files Patch % Lines
src/pylhe/__init__.py 90.00% 1 Missing and 7 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #233 +/- ## ========================================== - Coverage 91.48% 91.17% -0.32% ========================================== Files 2 2 Lines 235 306 +71 Branches 54 78 +24 ========================================== + Hits 215 279 +64 - Misses 16 17 +1 - Partials 4 10 +6 ``` | [Flag](https://app.codecov.io/gh/scikit-hep/pylhe/pull/233/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | Coverage Δ | | |---|---|---| | [unittests-3.10](https://app.codecov.io/gh/scikit-hep/pylhe/pull/233/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `90.84% <90.00%> (-0.22%)` | :arrow_down: | | [unittests-3.8](https://app.codecov.io/gh/scikit-hep/pylhe/pull/233/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `90.84% <90.00%> (-0.22%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

APN-Pucky commented 2 months ago

@APN-Pucky can you please add a summary commit message to the PR body that can be copy-pasted in for the squash and merge?

Done.

matthewfeickert commented 2 months ago

Done.

Thanks @APN-Pucky! Can you now rebase this off of main now that PR #232 is in (and fixup the merge conflict there)? After that I think this is good to go.

APN-Pucky commented 2 months ago

Thanks @APN-Pucky! Can you now rebase this off of main now that PR #232 is in (and fixup the merge conflict there)? After that I think this is good to go.

:heavy_check_mark:

matthewfeickert commented 2 months ago

Thanks @APN-Pucky. I'll leave this open for @eduardo-rodrigues to look at, but if he hasn't reviewed it by Friday 2024-08-22 I'll merge this.

We can then get a minor release out to PyPI as @agbuckley asked about in Discussion https://github.com/scikit-hep/pylhe/discussions/238.

agbuckley commented 2 months ago

Thanks!!

eduardo-rodrigues commented 2 months ago

Hi everyone. As you could guess I had been away and offline.

This is a really nice addition :+1!

My only comment to you @APN-Pucky is that the functionality should be visible as an example notebook for sure, probably in a dedicated new notebook (otherwise many will not realise writing is now available). Likewise I would add a little paragraph/sentence in the README.