-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add with_units_base
feature
#1232
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1232 +/- ##
==========================================
+ Coverage 84.29% 84.31% +0.02%
==========================================
Files 181 181
Lines 8391 8399 +8
==========================================
+ Hits 7073 7082 +9
+ Misses 1318 1317 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
LGTM
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.
Could you add an example at the end of this tutorial section showing with_units_base
? https://nrel-sienna.github.io/PowerSystems.jl/stable/tutorials/creating_system/#Changing-System-Per-Unit-Settings
and mention in https://nrel-sienna.github.io/PowerSystems.jl/stable/explanation/per_unit/ with the other functions (halfway down). Thanks!
@kdayday fixed the docstrings and added to that tutorial and that explanation page — how do those look? |
@GabrielKS can you rebase this branch to update it? |
efefbf7
to
6e8e2f2
Compare
Done |
Closes #1229 by adding a new exported function
with_units_base
that serves as a "context manager" to execute a block of code as if the system's units base were set to a particular value.