-
Notifications
You must be signed in to change notification settings - Fork 23
feat: dataclass implementation #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #307 +/- ##
==========================================
+ Coverage 95.10% 95.82% +0.72%
==========================================
Files 2 2
Lines 347 407 +60
Branches 63 59 -4
==========================================
+ Hits 330 390 +60
Misses 10 10
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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 converts the LHE data classes from dict-based implementations to proper dataclasses for improved type safety and clarity. The implementation maintains backwards compatibility through dictionary-like access methods and properties.
- Converts LHEEvent, LHEEventInfo, LHEParticle, LHEInitInfo, LHEProcInfo, LHEInit, and LHEFile to dataclasses with proper type annotations
- Adds backwards compatibility methods (getitem, setitem) with deprecation warnings for dict-like access
- Changes LHEVersion from float to string type and updates related tests
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/pylhe/init.py | Core implementation converting classes to dataclasses with backwards compatibility methods |
tests/test_classes.py | Updated tests for TypeError exceptions, added backwards compatibility tests, and dataclass comparison updates |
tests/test_lhe_reader.py | Updated LHEVersion test assertion from float to string comparison |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
I quickly drafted a demo implementation of dataclasses instead of dicts. I considered
__setitem__
and__getitem__
to mimic olddict
-behavior like e.g.self["beamA"]
, but with a deprecation warning in order to get people to change their code.RuntimeError
->TypeError
dict
anymore we can not directly compare against adict
, however thedataclass.asdict
can be used in the tests now.LHEInit
andLHEProcInfo
previously could be instantiated without parameters, but I don't think that makes sense. Turning all their dataclass fields to optional also is ugly in my eyes.@property
that reads the fields.TODOs and considerations:
fieldnames()
property and__setitem__
/__getitem__
.LHEVersion
should not be a float (stupid AI...)LHEVersion
fieldnames()
property to be the same as previouslyCloses: #303