-
Notifications
You must be signed in to change notification settings - Fork 25
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
F.Traps
and F.DerivedQuantities
-> subclasses of list
#697
F.Traps
and F.DerivedQuantities
-> subclasses of list
#697
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #697 +/- ##
==========================================
+ Coverage 99.44% 99.47% +0.02%
==========================================
Files 58 58
Lines 2176 2280 +104
==========================================
+ Hits 2164 2268 +104
Misses 12 12 ☔ View full report in Codecov by Sentry. |
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.
@KulaginVladimir thanks for this! almost there!
I'm just realising that having the property for .traps (for instance) doesn't break the api if users do:
print(my_traps.traps[2])
but users can't do
my_traps.traps.append(...)
Can you please add a setter/getter in addition to the @Property please?
test/unit/test_exports/test_derived_quantities/test_derived_quantities.py
Outdated
Show resolved
Hide resolved
test/unit/test_exports/test_derived_quantities/test_derived_quantities.py
Show resolved
Hide resolved
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.
Thanks for this @KulaginVladimir the code is much cleaner now!
Proposed changes
This PR modifies
F.Traps
andF.DerivedQuantities
to be subclasses of list following #494.An attempt was made to add a check that:
F.Materials
/F.Exports
/F.Traps
/F.DerivedQuantities
F.Material
/F.Export
/F.Trap
/F.DerivedQuantity
Additionally, a
.materials
/.exports
/.traps
/.derived_quantities
property is temporarly added toF.Materials
/F.Exports
/F.Traps
/F.DerivedQuantities
to minimize a breaking change, as privately discussed with @RemDelaporteMathurin.Types of changes
What types of changes does your code introduce to FESTIM?
Checklist