-
Notifications
You must be signed in to change notification settings - Fork 232
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
Adding UnitSet object to manage definition of base and derived units #991
Conversation
I should also write some docs for the new UnitSet, but I think these don;t need to be too extensive as this is mostly an internal object that users won't see much. |
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.
A couple questions (I think we can make this implementation simpler)...
Codecov ReportBase: 69.98% // Head: 70.04% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #991 +/- ##
==========================================
+ Coverage 69.98% 70.04% +0.05%
==========================================
Files 400 400
Lines 66171 66275 +104
Branches 12204 12201 -3
==========================================
+ Hits 46308 46420 +112
+ Misses 17486 17484 -2
+ Partials 2377 2371 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@dangunter @jsiirola I think this is ready for the next round of reviews, and is hopefully almost ready to merge (I'll save updating the rest of the code base for a separate PR). |
Part of #669
Summary/Motivation:
As a start towards removing a number of "magic string constants" in the IDAES codebase, this PR introduces a new
UnitSet
object which defines the set of known quantities in IDAES as properties along with their define units of measurement. For backwards compatibility,UnitSets
allow quantities to be accessed in a dict-like fashion using a string for the quantity name, but as this is rolled out we will begin replacing instances of this in the codebase with direct references to the properties on theUnitSet
.Changes proposed in this PR:
UnitSet
object, and replaces existent_default_units
and_derived_units
dicts with this inproperties_meta
.UnitSets
.UnitSets
instead.Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: